search

JFXtras / jfxtras-styles

Java, JavaFX themes or look and feels. Currently contains JMetro theme.
https://pixelduke.com/java-javafx-theme-jmetro/
643 stars 144 forks source link

Hide cross for uneditable TextFields (disabled != uneditable) #218

Closed satsen closed 2 years ago

satsen commented 2 years ago

Fixes #209 Fixes #204

Actually there was an issue (#180) and pull request made for this 1.5 years ago (#182) but because the author didn't explain which case wasn't covered previously, they got closed.

When you have a TextField with setEditable(false) (which keeps the same text style but makes it uneditable and is not the same as setDisable(true) which also makes it gray and unselectable), the cross is still there. This PR fixes that.

satsen commented 2 years ago

By the way, I also deleted the empty jmetro--samples (with two dashes) folder. I also added Gradle & Maven dependency snippets. Just trying to fix up the repository a little :+1: Hope you can do a release after you merge the current open pull requests as well :)

satsen commented 2 years ago

@dukke

satsen commented 2 years ago

@dukke

dukke commented 2 years ago

Hi @satsen

Truly sorry for the late reply. I've been very busy with my work.

Can we separate each PR into its own issue? That is, each PR solving just one issue.

Thanks and sorry again..

dukke commented 2 years ago

By the way, I also deleted the empty jmetro--samples (with two dashes) folder. I also added Gradle & Maven dependency snippets. Just trying to fix up the repository a little 👍 Hope you can do a release after you merge the current open pull requests as well :)

The "jmetro-samples" folder exists for the master branch the other folder (jmetro--samples) is for the jdk-8 branch

dukke commented 2 years ago

.. having a PR for each separate issue is important because then we can discuss each issue individually and it's not an all or nothing approach. For instance, the uneditable textfield issue could be merged immediately but because there are other things being changed in this PR that need to be discussed, it can't be merged.

dukke commented 2 years ago

All these has already been fixed.

I've put a reference to you on one of the commits. Thanks for your contribution.

satsen commented 2 years ago

@dukke Well it's unfortunate that this PR didn't get merged, I guess you could have edited the offending commit in the PR but yeah it is my fault and you should not have to spend time on fixing my mistake for adding unrelated changes so I am sorry. At least I'm glad that the code helped, even if the PR did not.

Sorry for late reply as well. And I'm glad that you're back and made a new release

dukke commented 2 years ago

No it's not your fault @satsen . I (and the users of JMetro) just have to thank you for your contribution.

Unfortunately I wasn't having time to work on JMetro, if I had I would have commented earlier and you would have made the change. Anyways, I put a reference to your username in the commit and there's always the chance that you'll file another PR that will get merged 🙂