Skip to content

Fixed SettingNumeric textinput validation - #9119

Open
takkasila wants to merge 1 commit into
kivy:masterfrom
takkasila:issue_fix-settings-numeric-validate
Open

takkasila wants to merge 1 commit into
kivy:masterfrom
takkasila:issue_fix-settings-numeric-validate

Conversation

@takkasila

@takkasila takkasila commented Aug 17, 2025

Copy link
Copy Markdown

Bug fix for the issue #9095

  • Fixed SettingNumeric textinput validation.
  • Added unittest for the fix.

Maintainer merge checklist

  • Title is descriptive/clear for inclusion in release notes.
  • Applied a Component: xxx label.
  • Applied the api-deprecation or api-break label.
  • Applied the release-highlight label to be highlighted in release notes.
  • Added to the milestone version it was merged into.
  • Unittests are included in PR.
  • Properly documented, including versionadded, versionchanged as needed.

- Added unittest for the fix
@welcome

welcome Bot commented Aug 17, 2025

Copy link
Copy Markdown

Thanks for opening your first pull request here! 💖 Please check out our contributing guidelines.

@kuzeyron kuzeyron added the Component: core-app app, clock, config, inspector, logger, resources, modules, clock, base.py label Aug 19, 2025

@pytest.fixture(autouse=True, scope="function")
def setup(self):
self.settingNumeric = SettingNumericTest(self.panel)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am concerned here. You are actually testing a subclass of SettingNumeric, not an instance of SettingNumeric itself. You've added a bunch of properties which could interfere with it's behaviour. Even if not now, it could in future versions. Rather build the test to test a pure SettingNumeric instance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Component: core-app app, clock, config, inspector, logger, resources, modules, clock, base.py

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants