Skip to content
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

feat(tags): support removable #1266

Merged
merged 11 commits into from May 30, 2022
Merged

feat(tags): support removable #1266

merged 11 commits into from May 30, 2022

Conversation

yinonov
Copy link
Contributor

@yinonov yinonov commented Mar 29, 2022

issues to resolve:

  • tags wrapper to observe assigned nodes (measure performance of textContent tags instances / manage state of assigned tags) @YonatanKra
  • consider removing dense / enlarge if not required at this point @jshenkman
  • research focus management in accordance to keyboard control @yinonov
  • should remove button feature ripple as well? @rachelbt
  • should selected feature ripple? @rachelbt

image

@github-actions
Copy link

🚀

Latest successful build of the PR deployed here.

🚀

@sonarcloud
Copy link

sonarcloud bot commented Mar 29, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@yinonov yinonov linked an issue Mar 29, 2022 that may be closed by this pull request
@yinonov yinonov added the good first issue Good for newcomers label May 10, 2022
@yinonov yinonov added the help wanted Extra attention is needed label May 10, 2022
@yinonov
Copy link
Contributor Author

yinonov commented May 10, 2022

will handle tag open issues in a different scope

@yinonov yinonov marked this pull request as ready for review May 10, 2022 19:18
@yinonov yinonov mentioned this pull request May 10, 2022
5 tasks
Copy link
Contributor

@rachelbt rachelbt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there's no option for both selected & removable. Is it intentionally?
There's no removable option in the UI test

components/tags/src/vwc-tag.scss Outdated Show resolved Hide resolved
components/tags/src/vwc-tag.scss Show resolved Hide resolved
yinonov and others added 2 commits May 18, 2022 13:20
@yinonov
Copy link
Contributor Author

yinonov commented May 19, 2022

there's no option for both selected & removable. Is it intentionally? There's no removable option in the UI test

actually that's a good question. I don't think they should co-exist but it's not specified. let's try to monitor public feedback and "field" testing. semantics & a11y concerns should be researched here

@rachelbt
Copy link
Contributor

I think we need to add wither just cursor pointer to the button or both that & ripple to indicate that the X is a button and not decorative

@yinonov
Copy link
Contributor Author

yinonov commented May 19, 2022

I think we need to add wither just cursor pointer to the button or both that & ripple to indicate that the X is a button and not decorative

It is addressed in different issues

@sonarcloud
Copy link

sonarcloud bot commented May 30, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@yinonov yinonov merged commit 5b3cd3c into master May 30, 2022
@yinonov yinonov deleted the 1264-tag-support-removable branch May 30, 2022 12:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers help wanted Extra attention is needed
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[tag] support removable
2 participants