Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update HasLabelStringMatcher.java #108

Merged
merged 1 commit into from Jul 14, 2014
Merged

Update HasLabelStringMatcher.java #108

merged 1 commit into from Jul 14, 2014

Conversation

torakiki
Copy link
Contributor

Class name suggests this matcher should match only Labeled but the private nodeHasLabel also handles TextInputControl. If this is the intended behavior I think matchesSafetly should be changed to accept TextInputControl as well

Class name suggests this matcher should match only Labeled but the private nodeHasLabel also handles TextInputControl. If this is the intended behavior I think matchesSafetly should be changed to accept TextInputControl as well
@hastebrot
Copy link
Member

Hey! Thanks for the PR.

Yes, there are some name inconsistencies in the Matchers, e. g. Commons#hasLabel() and Commons#hasText() both use HasLabelStringMatchers.

I tinkered a lot about how to improve the naming and started to refactor the Matchers. For example I think we should put Matchers related to the Labeled control into LabeledMatchers. Those related to TextInputControl into TextInputControlMatchers. Since it is very convenient to just use a single Matcher for both Labeled and TextInputControl there should be a NodeMatchers with hasText() that works for both of them.

Also these Matchers will be typed and fail with an error message if the wrong control that uses Node as superclass is tested.

hastebrot added a commit that referenced this pull request Jul 14, 2014
Update HasLabelStringMatcher.java
@hastebrot hastebrot merged commit a9288cb into TestFX:master Jul 14, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants