-
Notifications
You must be signed in to change notification settings - Fork 212
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
Cleanup tag display for long lists of tags #3808
Conversation
f6421a0
to
d73b71e
Compare
adbcdc6
to
dd444dc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This works so well 🎉 I made one suggestion for code quality, but functionality-wise this is awesome.
651462d
to
2d053e4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@obulat I've drafted this so we can make the changes discussed in Slack:
- Add a
TOGGLE_TAG_EXPANSION
(or similar name) analytics event - Finalize the language
c0db3fe
to
02c57f7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! I have a few suggestions but I will not block this PR.
Signed-off-by: Olga Bulat <[email protected]>
Signed-off-by: Olga Bulat <[email protected]>
Signed-off-by: Olga Bulat <[email protected]>
Signed-off-by: Olga Bulat <[email protected]>
Signed-off-by: Olga Bulat <[email protected]>
Signed-off-by: Olga Bulat <[email protected]>
Signed-off-by: Olga Bulat <[email protected]>
Signed-off-by: Olga Bulat <[email protected]>
Signed-off-by: Olga Bulat <[email protected]>
Signed-off-by: Olga Bulat <[email protected]>
Signed-off-by: Olga Bulat <[email protected]>
f6450e1
to
4ccbaf2
Compare
Fixes
Fixes #2589 by @zackkrida
Description
This PR hides the tags beyond the third row and shows a "Show more tags" button if there are hidden tags.
I first tried simply hiding the tags setting the max height and using
overflow-hidden
. However, this way, you can navigate to the hidden tags using Tab. Even though those tags are not visible, they are focusable and are read by the screenreader.That's why I added a function that detects the index of the tag that is the first on the 4th line of tags, and set the
visibleTags
value to the tags up to the "first on the 4th line". When the user clicks show more, thevisibleTags
is set to all of the tags, and when they click on hide tags, thevisibleTags
array only contains the first 3 lines.For counting the rows of tags, I used
offsetLeft
, but had to make comparison operator (<
or>
) conditional to make it work with RTL (some of the first VR tests here for RTL are failures because I only used one comparison)The expander button has a
aria-expanded
set totrue
orfalse
based on whether the tags are all shown, or hidden.Testing Instructions
Enable the
additional_search_views
on the/preferences
page (note that this is only implemented for when this flag is on)Go to an image result with many tags, such as
[http://localhost:8443/image/7e138814-e774-45fb-91ee-7b728c174616?q=Titmouse](http://localhost:8443/image/7e138814-e774-45fb-91ee-7b728c174616?q=Titmouse)
.Check that only the 3 rows of tags are displayed, with a "Show more tags" button. Try clicking on the button. The first opened tag should be focused, and the "See less" button should be displayed.
Check that this works for pages with other locales, and that the tags for media with fewer tags are displayed as before.
Checklist
Update index.md
).main
) or a parent feature branch.Developer Certificate of Origin
Developer Certificate of Origin