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

fix(component): display text only on Sidebar.Item tooltip #315

Merged
merged 2 commits into from
Jul 31, 2022

Conversation

tulup-conner
Copy link
Collaborator

@codecov
Copy link

codecov bot commented Jul 31, 2022

Codecov Report

Merging #315 (e665b42) into main (77cb356) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #315      +/-   ##
==========================================
- Coverage   99.13%   99.12%   -0.01%     
==========================================
  Files         133      133              
  Lines        8276     8252      -24     
  Branches      955      947       -8     
==========================================
- Hits         8204     8180      -24     
  Misses         72       72              
Impacted Files Coverage Δ
src/lib/components/Sidebar/Sidebar.spec.tsx 100.00% <ø> (ø)
src/lib/components/Sidebar/SidebarItem.tsx 100.00% <100.00%> (ø)
src/lib/components/Sidebar/index.tsx 100.00% <100.00%> (ø)
src/lib/theme/default.ts 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 77cb356...e665b42. Read the comment docs.

@tulup-conner tulup-conner marked this pull request as draft July 31, 2022 01:03
@tulup-conner tulup-conner marked this pull request as ready for review July 31, 2022 19:38
@tulup-conner
Copy link
Collaborator Author

I've deliberately removed tests in this PR. These tests are unreliable. We should use cypress to test any hover interactions.

When a `Sidebar` is collapsed, each item's text content is hidden. You hover over an item to see a
`Tooltip` with its text content. That isn't how it worked up until now.

resolve themesberg#258
I don't think we can rely on a fake DOM environment to test hover states correctly. We should write
integration tests for `<Sidebar collapsed/>` behavior in `cypress`.
Copy link
Collaborator

@rluders rluders left a comment

Choose a reason for hiding this comment

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

lgtm

@rluders rluders merged commit 9af5d13 into themesberg:main Jul 31, 2022
@tulup-conner tulup-conner deleted the fix/sidebar-collapsed-hover branch July 31, 2022 22:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants