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

Add box-shadow back to the Sensei tour #7543

Merged
merged 5 commits into from
Mar 19, 2024

Conversation

renatho
Copy link
Contributor

@renatho renatho commented Mar 15, 2024

Resolves #7504
Based off of #7538

Proposed Changes

  • After thinking about the proposed options in the issue, I decided to go with the one to hide metaboxes. Making something with a dynamic position doesn't seem something safe to handle without issues, and changing the position to the top I don't think would be nice.
  • I think this approach is nice like a "focus mode" when the tour is open explaining the features.
  • It adds a box-shadow to the Sensei tour, so it's better visible with a white background.

Testing Instructions

  1. With Sensei Pro activated, open the lesson editor with the quiz block added.
  2. Make sure that when the Tour is open, and you can clearly see the tour limited through a box shadow over the content drip that is white.

Pre-Merge Checklist

  • PR title and description contain sufficient detail and accurately describe the changes
  • Acceptance criteria is met
  • Decisions are publicly documented
  • Adheres to coding standards (PHP, JavaScript, CSS, HTML)
  • All strings are translatable (without concatenation, handles plurals)
  • Follows our naming conventions (P6rkRX-4oA-p2)
  • Hooks (p6rkRX-1uS-p2) and functions are documented
  • New UIs are responsive and use a mobile-first approach
  • New UIs match the designs
  • Different user privileges (admin, teacher, subscriber) are tested as appropriate
  • Legacy courses (course without blocks) are tested
  • Code is tested on the minimum supported PHP and WordPress versions
  • User interface changes have been tested on the latest versions of Chrome, Firefox and Safari
  • "Needs Documentation" label is added if this change requires updates to documentation
  • Known issues are created as new GitHub issues

@renatho renatho added this to the 4.22.0 milestone Mar 15, 2024
@renatho renatho self-assigned this Mar 15, 2024
@renatho renatho marked this pull request as ready for review March 15, 2024 20:56
@renatho renatho changed the base branch from trunk to add/interaction-on-lesson-step March 15, 2024 20:56
@renatho renatho requested a review from a team March 15, 2024 20:56
Copy link

Test the previous changes of this PR with WordPress Playground.

Copy link

github-actions bot commented Mar 15, 2024

WordPress Dependencies Report

The github-action-wordpress-dependencies-report action has detected some script changes between the commit 57b44ea and trunk. Please review and confirm the following are correct before merging.

No changes detected in the current commit. But the comment was left so it is possible to check for the edit history.

This comment was automatically generated by the github-action-wordpress-dependencies-report action.

Copy link

codecov bot commented Mar 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 51.78%. Comparing base (da17671) to head (57b44ea).
Report is 19 commits behind head on trunk.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##              trunk    #7543   +/-   ##
=========================================
  Coverage     51.78%   51.78%           
  Complexity    11307    11307           
=========================================
  Files           640      640           
  Lines         48115    48115           
  Branches        467      467           
=========================================
  Hits          24914    24914           
  Misses        22822    22822           
  Partials        379      379           

see 6 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

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

Imran92
Imran92 previously approved these changes Mar 18, 2024
Copy link
Contributor

@Imran92 Imran92 left a comment

Choose a reason for hiding this comment

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

LGTM!

@donnapep
Copy link
Collaborator

@Imran92 @renatho It seems this would be better handled by adding a box shadow (or some other distinguishing UI) to the tour as discussed here - p1710793336101129/1710779204.883429-slack-C02NWDZBL0H

Base automatically changed from add/interaction-on-lesson-step to trunk March 19, 2024 15:22
@renatho renatho dismissed Imran92’s stale review March 19, 2024 15:22

The base branch was changed.

@renatho
Copy link
Contributor Author

renatho commented Mar 19, 2024

@donnapep and @Imran92 , WDYT about this? 5dd7c35

Screenshot 2024-03-19 at 14 14 40

It adds back a box-shadow removed by wpcom class.

@renatho renatho changed the title Hide metaboxes when Tour is being displayed Add box-shadow back to the Sensei tour Mar 19, 2024
Copy link

Test the previous changes of this PR with WordPress Playground.

@@ -19,8 +23,7 @@ $border-color: var(--wp--preset--color--primary, currentcolor);
box-shadow: 0 0 $shadow-width $border-color !important;

&.sensei-tour-highlight--inset {
outline: none !important;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here I just took the opportunity to remove this property that is not needed anymore after #7548. And I also removed the double ;.

Copy link

Test the previous changes of this PR with WordPress Playground.

@donnapep
Copy link
Collaborator

It adds back a box-shadow removed by wpcom class.

Looks great! ⭐ It also looks good when the theme's background is white:

Screenshot 2024-03-19 at 2 13 09 PM

@renatho renatho merged commit ea662f1 into trunk Mar 19, 2024
25 checks passed
@renatho renatho deleted the add/metaboxes-position-conflict-handler branch March 19, 2024 19:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Handle showing the Lesson Quiz tour modal when Content Drip panel is at the bottom
3 participants