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

Tooltip with long text cuts off when hovering over wins icons #2408 #2644

Conversation

JimGeist
Copy link
Member

Fixes #2408

What changes did you make and why did you make them ?

The text bubble was appearing below the WINS badge icon and the text was getting off when the badge description had multiple lines. The solution was to have the bubble appear above the WINS badge icon. The changes were made in _sass\components\_wins-page.scss

  • .wins-text-bubble : added bottom: 50px; to the declaration at line 95. bottom: 50px causes the bottom of the bubble to appear 50px above the bottom of the icon container. 50px give a small gap between the bubble and the icon.
  • .wins-text-bubble:before (lines 108, 110-111): This is the css for the little triangle 'arrow'.
    • line 108: Transform was changed from transform: rotate(45deg) to transform: rotate(225deg). 45deg had the triangle pointing up (apex on top) 2408_0-Before-arrow which worked when the bubble was below the WINS icon with the 'arrow' pointing up at the icon. Since the bubble is now above the icon, the 'arrow' needs to point down at the icon 2408_1-After-arrow via the 225deg rotation.
    • right: 40% was changed to right: 47%. This positions the 'arrow' close to the horizontal center of the WINS icon.
    • top: 0 was changed to bottom: 0 because the 'arrow' needs to appear at the bottom of the bubble.
  • .wins-table > .wins-test-bubble in @media #{$bp-below-desktop}: line 494 NEW - issue was reopened because on smaller width devices, the text for the WINS was vertically shifted 50px because for smaller device widths, the WINS card expands vertically to show all the WINS badges (instead of the modal window) when a badge is clicked.

I am not sure if having the text bubble temporarily appear over existing text is an acceptable practice. Altering the padding for the WINS card would have affected the appearance of the page by created too much negative space on the bottom of the card.

The changes were tested with browser window sizes > 960px, < 960px, ~380px, and Responsive modes for mobile devices and tablets to ensure the display is correct. The only anomaly (and it exists on the current production site) is when the window is exactly 960px wide. The WINS card expands and the WINS badges are in 2 columns but without the text. 960px is the minimum desktop width and changing variables/_layout.scss to fix this anomaly may cause display issues in other code.

Screenshots of Proposed Changes Of The Website (if any, please do not screen shot code changes)

Visuals before changes are applied

2408_0-Before_WINS

Shifted text in the expanded WINS card (initial push of this fix, the production site does NOT look like this)
2408_2-BeforeReopen_WINS

Visuals after changes are applied

2408_1-After_WINS

2408_2-After_WINS

@github-actions
Copy link

Want to review this pull request? Take a look at this documentation for a step by step guide!

From your project repository, check out a new branch and test the changes.

git checkout -b JimGeist-tooltip-with-long-text-cuts-off-in-wins-2408 gh-pages
git pull https://github.com/JimGeist/website.git tooltip-with-long-text-cuts-off-in-wins-2408

@github-actions github-actions bot added Bug Something isn't working P-Feature: Wins Page https://www.hackforla.org/wins/ role: front end Tasks for front end developers Complexity: Medium Status: Updated No blockers and update is ready for review labels Dec 30, 2021
@sydneywalcoff sydneywalcoff self-requested a review December 31, 2021 17:09
@sydneywalcoff
Copy link
Member

Review ETA: Sunday Jan 2, 2022

Copy link
Member

@sydneywalcoff sydneywalcoff left a comment

Choose a reason for hiding this comment

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

The changes were tested with browser window sizes > 960px, < 960px, ~380px, and Responsive modes for mobile devices and tablets to ensure the display is correct. The only anomaly (and it exists on the current production site) is when the window is exactly 960px wide. The WINS card expands and the WINS badges are in 2 columns but without the text. 960px is the minimum desktop width and changing variables/_layout.scss to fix this anomaly may cause display issues in other code.

Regarding this, it seems like this might end up its own issue, but just to note that when clicked, at 960px the text appears in a bubble rather than displayed next to the icon.

Otherwise, LGTM. Good job Jim!

Copy link
Member

@R-Tomas-Gonzalez R-Tomas-Gonzalez left a comment

Choose a reason for hiding this comment

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

Hey @JimGeist, I'm holding off on this till I can speak with you. I think we may have another solution that may work, which is to have the tooltip float below the wins icon, and on top of the card/whitespace. This change would be on the .project-card css style. Please let me know if you have tried this already. This may also be a question for design to see what they prefer. Sorry for taking so long to get this issue to push out. If you'd like, please pick up a new issue, while keeping this one in your backlog. Before implementing any other changes on this issue, let's aim to get a clear answer tomorrow.
Thanks,
-Tomas

@JimGeist
Copy link
Member Author

JimGeist commented Jan 9, 2022

Tomas, Thank you for the update, but realize that the current code addresses an existing bug and should be pushed and no longer held off. It is fine if you are seeking a different solution. That should not, however, hold up a fix that already addresses the cut off text issue.

Copy link
Member

@R-Tomas-Gonzalez R-Tomas-Gonzalez left a comment

Choose a reason for hiding this comment

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

Hey Jim, I understand your frustration. Because this is a bug that concerns design and the wins page, your fix would have been more difficult to undo if we were all to agree with a different solution — which we have. We talked about this in our Sunday meeting and have all agreed that another solution would suffice for this issue.

The new recommended solution is to take off the overflow attribute in the stylesheet.
That is:

Please contact me via slack or via github with any questions or concerns. Thank you.

@R-Tomas-Gonzalez
Copy link
Member

Closing this pull request as personal issues arose, and alternate route was asked from design, pm and dev teams. Thanks for your work on this @JimGeist

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working Complexity: Medium P-Feature: Wins Page https://www.hackforla.org/wins/ role: front end Tasks for front end developers Status: Updated No blockers and update is ready for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tooltip with long text cuts off when hovering over wins icons
3 participants