fix: allow text in read-only components to be selected#4417
fix: allow text in read-only components to be selected#4417TrevorBurnham wants to merge 2 commits intocloudscape-design:mainfrom
Conversation
227fc00 to
119d85c
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #4417 +/- ##
=======================================
Coverage 97.38% 97.38%
=======================================
Files 928 928
Lines 29200 29200
Branches 10565 10564 -1
=======================================
Hits 28435 28435
- Misses 718 759 +41
+ Partials 47 6 -41 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Would it be possible to add coverage, maybe with the Selection API? https://developer.mozilla.org/en-US/docs/Web/API/Selection |
|
|
||
| // read-only in-filtering-token (i.e. for property filter readOnlyOperations) | ||
| // should retain standard borders, only the icon changes | ||
| &.readonly:not(&.disabled):not(&.in-filtering-token) { |
There was a problem hiding this comment.
Note: probably a minor use case, but the :not(&.in-filtering-token) excludes from this fix read-only operator selectors in property filter:
![]()
At the same time, we would prefer placeholder text to not be selectable, because that comes with collateral a11y issues (if it is interactive, it will have other requirements such as contrast):
There was a problem hiding this comment.
I've moved the user-select: text to a separate &.readonly:not(&.disabled) rule that also covers filtering tokens. I've also added user-select: none to .placeholder.
- Apply `user-select: text` to all read-only button-triggers (including those inside property-filter tokens, i.e. `readOnlyOperations`), while keeping `form-readonly-element` borders scoped to the non-filtering case. Fixes the gap where read-only property-filter operator text was still unselectable. - Set `user-select: none` on the custom placeholder spans in Select (`.placeholder`) and DateRangePicker (`.label-text`). When these components are read-only without a value, the trigger renders placeholder text; preserving its non-selectable nature avoids accessibility concerns that accompany selectable/interactive text. - Add a DateRangePicker integration test that programmatically selects the read-only trigger's contents via the Selection API and asserts the selected string contains the rendered date values.
8de254f to
2998c7c
Compare
|
I've rebased the branch, added test coverage (see the new "Read-only mode" describe block), and addressed your comments |
Fixes #4411
This PR modifies the styles on
DateRangePicker,FileTokenGroup,SelectandMultiselectto make text selectable.Testing
I've added a read-only toggle to the demo pages and was able to manually test each of these.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.