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] Fix content and footer padding #44104

Merged
merged 7 commits into from
Sep 18, 2024

Conversation

obuisard
Copy link
Contributor

@obuisard obuisard commented Sep 17, 2024

Pull Request for Issue #44096.

Summary of Changes

When footer buttons are missing, the step content has no bottom padding.

Testing Instructions

There is no core tour that can show this behavior.

Create a new tour (with relative URL: administrator/index.php, Component selector: 'Home dashboard').

Create the following steps for the tour:

Step 1:
Position: Bottom
Target: .quickicon a[href*=com_config]
Type: Interactive
Interactive type: Form submit

Step 2:
Position: Bottom
Target: #toolbar-save
Type: Interactive
Interactive type: Form submit

Step 3:
Position: Center
Type: Next

It does not matter what you enter in titles and descriptions.

Run the tour.

Run other tours and make sure bottom padding is correct in the other tour(s) you run.

Actual result BEFORE applying this Pull Request

No padding at the bottom of the popup in step 3.

image

Expected result AFTER applying this Pull Request

Padding at the bottom of the popup in step 3.

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 Sep 17, 2024
@Kostelano
Copy link
Contributor

I have tested this item ✅ successfully on a38c01b

It works, it's been tested.

The test instructions are not complete, after step 2 you need to create another step - informational, since step 2 of the test instructions (if it is the final one) will have a Close button, and we need to get a step without buttons.

In addition, the PR can be tested on an existing tour - a tour for creating STEPS (specifically step 3).


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

@richard67
Copy link
Member

richard67 commented Sep 17, 2024

@obuisard node build/build.js --prepare fails in Drone. It seems there is something wrong with the CSS, but I don't see any mistake.
I will try if a branch update helps.

@obuisard
Copy link
Contributor Author

I have tested this item ✅ successfully on a38c01bIt works, it's been tested.

The test instructions are not complete, after step 2 you need to create another step - informational, since step 2 of the test instructions (if it is the final one) will have a Close button, and we need to get a step without buttons.

Thanks for the correction, you are right, it needs an extra step. I had one in my test tour.

@obuisard
Copy link
Contributor Author

@obuisard node build/build.js --prepare fails in Drone. It seems there is something wrong with the CSS, but I don't see any mistake. I will try if a branch update helps.

Got it, it does not like the order of the properties...

@richard67
Copy link
Member

@Kostelano Could you briefly test again if it still works? I don't expect any issues but I'd like to be safe. Thanks in advance.

@Kostelano
Copy link
Contributor

I have tested this item ✅ successfully on bcc1df9


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

@obuisard
Copy link
Contributor Author

obuisard commented Sep 17, 2024

I followed the requested change in Drone, but apparently it is not 'smart' enough to tell us the whole story. I had to change the order again. I apologize (I am not sure where to find information about what the order should be, as there is no standard for CSS property order).

@tecpromotion
Copy link
Contributor

I have tested this item ✅ successfully on 0abeffd


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

@richard67
Copy link
Member

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Sep 17, 2024
@tecpromotion
Copy link
Contributor

Step 3 in "How to add steps to a guided tour?" now with padding.
/administrator/index.php?option=com_guidedtours&view=steps&tour_id=12
step3-with-padding

@Hackwar Hackwar merged commit f08a6ab into joomla:5.2-dev Sep 18, 2024
4 checks passed
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Sep 18, 2024
@Hackwar Hackwar added this to the Joomla! 5.2.0 milestone Sep 18, 2024
@Hackwar
Copy link
Member

Hackwar commented Sep 18, 2024

Thanks for your contribution @obuisard!

@obuisard obuisard deleted the guided-tours-paddingfix branch October 29, 2024 20:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Guided Tours 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