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

[5.2] [Guided Tours] Arrows do not show properly, especially in dark mode #43825

Merged
merged 9 commits into from
Aug 9, 2024

Conversation

obuisard
Copy link
Contributor

@obuisard obuisard commented Jul 23, 2024

Summary of Changes

When introducing dark mode, a border was added to the tour popups so that the tours would be more visible in dark mode.
Unfortunately, this 'cuts' the arrows off the popup and no border was added to the arrows themselves so that they are not visible in dark mode.

This is the first attempt in fixing that.

Although this PR fixes the arrows, there is more to be done, particularly that the colors for the tour popups are too dark and barely pop over the content. Check PR#43854.

Testing Instructions

Run the welcome tour in light and dark mode and check that the arrows are showing properly.

Actual result BEFORE applying this Pull Request

A popup in light mode.
The popup border shows over the arrow and the arrow is not of the color of the header background.

light_before

A popup in dark mode.
The border cuts off the arrow from the popup and is barely visible.

dark_before

Expected result AFTER applying this Pull Request

A popup in light mode.
The border is gone and the arrow is of the right color.

light_after

A popup in dark mode.
The border no longer cuts off the arrow from the popup and the arrow is more visible.

image

image

Link to documentations

Please select:

  • Documentation link for docs.joomla.org:

  • No documentation changes for docs.joomla.org needed

  • Pull Request link for manual.joomla.org:

  • No documentation changes for manual.joomla.org needed

@joomla-cms-bot joomla-cms-bot added NPM Resource Changed This Pull Request can't be tested by Patchtester PR-5.2-dev labels Jul 23, 2024
@Quy
Copy link
Contributor

Quy commented Jul 24, 2024

I have tested this item ✅ successfully on da2ff39


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/43825.

1 similar comment
@LadySolveig
Copy link
Contributor

I have tested this item ✅ successfully on da2ff39


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/43825.

@Hackwar Hackwar added the RTC This Pull Request is Ready To Commit label Jul 26, 2024
@obuisard
Copy link
Contributor Author

obuisard commented Jul 26, 2024

My sincere apology, I did not refresh my page and did not see the latest test and RTC label.
I reverted the unnecessary change in background color of the popup header in favor of PR#43854.
The change in color was minimal (from black to a lighter dark color) and had no effect on the arrow fixes I made in this PR.
Therefore the tests, when focusing on the arrow fix alone, will return the same exact results. I let it to the discretion of the release managers to decide if the tests need to be redone or not.
(My initial thought was to remove the color change to avoid merge issues later on...)

@pe7er pe7er self-assigned this Aug 8, 2024
@pe7er pe7er enabled auto-merge (squash) August 9, 2024 07:52
@pe7er pe7er merged commit ea9eedf into joomla:5.2-dev Aug 9, 2024
3 checks passed
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Aug 9, 2024
@pe7er
Copy link
Contributor

pe7er commented Aug 9, 2024

Thanks @obuisard !

@Quy Quy added this to the Joomla! 5.2.0 milestone Aug 9, 2024
@obuisard obuisard deleted the guided-tours-arrows branch October 29, 2024 20:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NPM Resource Changed This Pull Request can't be tested by Patchtester PR-5.2-dev
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants