Show filter hint only as placeholder in filtered viewers#4074
Conversation
vogella
force-pushed
the
filteredtree-no-initial-text
branch
from
3908dc3 to
46ae2b3
Compare
|
One thing I deliberately left alone: a few now-unreachable branches that compared the field value to initialText (the focusLost/mouseDown/focusGained handlers in this base, and the initial/narrowingDown checks in both FilteredTree copies). They're harmless dead branches now. I kept the diff minimal rather than ripping them out, can be done as follow-up clean-up if that change is accepted. |
There was a problem hiding this comment.
Pull request overview
Updates JFace’s AbstractFilteredViewerComposite#setInitialText so filtered viewers show the filter hint via Text#setMessage (placeholder) rather than writing the hint into the field’s actual value, improving UX/accessibility and removing the need for hint-as-value behavior.
Changes:
- Reworks
setInitialText(String)to set the filter field’s placeholder message instead of setting the field text. - Updates the
setInitialTextJavadoc to reflect placeholder semantics.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
vogella
force-pushed
the
filteredtree-no-initial-text
branch
from
46ae2b3 to
746950a
Compare
HeikoKlare
left a comment
HeikoKlare
left a comment
There was a problem hiding this comment.
In general, the change looks sounds to me. The method is used for providing the message to the filter input, for which it makes sense to not show the message as initial text input as well (as also done in #4044).
I have two remarks (even though I don't think we can do anything reasonable about that):
- The method is now quite misleading, as it is about setting the message and not about setting an initial text at all. This is an existing flaw (the method has the message in addition to the initial text before as well, without this really making sense), which becomes slightly more severe with this change.
textChanged()was always implicitly called when initializing anAbstractFilteredViewerCompositebefore this change. Consumers might rely on this behavior and break now.
I understand that it makes sense to keep this PR free from follow-up cleanups. What I would still propose it to clean up this code, as it will be hard to find that this change has affected it:
Since
FilteredTree_FilterMessage is not set as text input anywhere anymore (except for if a user accidentally inputs exactly that string), this check should be removed here, as it is tighly coupled to this value being set via setInitialText.
|
IIRC we have the same in the e4 FilteredTree so the diff will become a little bit bigger. Will update this as soon as I find time.... |
AbstractFilteredViewerComposite#setInitialText wrote the hint into the filter field value when the field had focus, which required clear-on-focus handling and could be misread by screen readers as an actual filter value. Expose the hint solely via Text#setMessage instead. This aligns FilteredTree, the e4 FilteredTree and FilteredTable with the approach already used by FilteredPreferenceDialog and ChooseWorkspaceDialog.
vogella
force-pushed
the
filteredtree-no-initial-text
branch
from
746950a to
5ee0060
Compare
|
Good point, done. Removed the now-dead |
|
This pull request changes some projects for the first time in this development cycle. An additional commit containing all the necessary changes was pushed to the top of this PR's branch. To obtain these changes (for example if you want to push more changes) either fetch from your fork or apply the git patch. Git patchFurther information are available in Common Build Issues - Missing version increments. |
|
@HeikoKlare is this fine for you? |
Filtered viewers used to write the hint text ("type filter text") into the filter field's value when the field had focus, which required clear-on-focus handling and could be read by screen readers as an actual filter value. This moves the shared
AbstractFilteredViewerComposite#setInitialTextto expose the hint solely as the field's placeholder viaText#setMessage.Showing a hint inside the field value is a recognized UX anti-pattern (see https://www.nngroup.com/articles/form-design-placeholders/). The placeholder gives the same visible hint without those drawbacks, and the accessible name is unaffected.
The fix lives in the JFace base, so it applies to
FilteredTree, the e4FilteredTreeandFilteredTableat once, aligning them with the approach already used byFilteredPreferenceDialogandChooseWorkspaceDialog.