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
VTag improvements #3975
VTag improvements #3975
Conversation
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.
I think the tests could use some cleanup but that it isn't blocking.
Co-authored-by: FelixSjogren <felixarvidsjogren@gmail.com> FEAT - Add lot for the content instead of a prop (#2) Closes #2 Co-authored-by: Stagge <jonatan.stagge@gmail.com> TEST - Add tests for VTag (#7) Added tests for Vtag, tests include: All props are sent to VButton VTag renders slot content Renders anchor tag. Co-authored-by: Stagge <jonatan.stagge@gmail.com> FEAT - Ensure inner VButton emits and handles events in VTag #4 Closes #4 Added accessibility label (#10) - Added aria-label to indicate that that the link is a tag Improvements from review lint Signed-off-by: Olga Bulat <obulat@gmail.com>
Signed-off-by: Olga Bulat <obulat@gmail.com>
dda343b
to
b585be0
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, I just have a doubt.
v-bind="$props" | ||
v-on="$listeners" |
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.
Why were these added?
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.
Normally, these should just pass through, but I think because we use 2 levels of components here: VButton
that is as
VLink
, they are not being passed without these.
Fixes
Fixes #3190 by @dhruvkb
This PR is the updated version of #3870 by @Stagge
Description
The VTag component added in #3137 has been improved in the following ways:
VButton
(and theVLink
component)VButton
Accessible labels to the links have been added to clearly indicate that the link is a tag.The a11y group advised not to add aria-labels, and use the inner text insteadTesting Instructions
Run the frontend app using
just frontend/run dev
Go to
http://localhost:8443/preferences
and switch theadditional_search_views
flag on.Go to the homepage and search for something, then select one of the results. You should see the tags work as expected.
There are no visual changes, so the VR tests pass.
Checklist
Update index.md
).main
) or a parent feature branch.Developer Certificate of Origin
Developer Certificate of Origin