Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Part of #1921152: META: Start providing tour tips for other core modules.
Problem/Motivation
Write tour integration for shortcuts page
Proposed resolution
Create tour yml files for shortcuts page
Remaining tasks
Provide patch of tour integration for the shortcuts page
User interface changes
New tours
API changes
None
Technical pointers when creating tour tips
See: https://drupal.org/node/1921152#tour-tips-tech-note for tech notes on making tour tips.
Comment | File | Size | Author |
---|---|---|---|
#61 | interdiff-2044407-59-61.txt | 1.31 KB | manuel.adan |
#61 | 2044407-61.patch | 3.92 KB | manuel.adan |
| |||
#53 | 2044407-53.patch | 3.97 KB | ridhimaabrol24 |
#52 | interdiff_50-52.txt | 740 bytes | ridhimaabrol24 |
#52 | 2044407-52.patch | 4.07 KB | ridhimaabrol24 |
Comments
Comment #1
nick_schuch CreditAttribution: nick_schuch commentedTour needs tests since we now have #2028535
Comment #2
nick_schuch CreditAttribution: nick_schuch commentedTagging for tracking.
Comment #3
jgardezi CreditAttribution: jgardezi commentedComment #4
jgardezi CreditAttribution: jgardezi commentedAdded shortcut patch
Comment #5
nielsonm CreditAttribution: nielsonm commentedChanged the text a little, got the tour items working. Still needs tests though.
Comment #6
benjy CreditAttribution: benjy commentedThis needs to be re-rolled to use routes now #1918768: Refactor tour module to use routes instead of paths is in.
Comment #7
Wim LeersComment #8
mr.baileysTackling this during the Belgian DUG sprint.
Comment #9
Ec1ipsis CreditAttribution: Ec1ipsis commentedAssigning to myself as issue has been inactive for two weeks.
Comment #10
Ec1ipsis CreditAttribution: Ec1ipsis commentedNew patch that updates shortcut Tour to use route instead of paths and adds additional tips to the shortcut Tour.
Comment #11
Ec1ipsis CreditAttribution: Ec1ipsis commentedComment #13
Ec1ipsis CreditAttribution: Ec1ipsis commented10: tour.shortcut-10.patch queued for re-testing.
Comment #14
aczietlow CreditAttribution: aczietlow commentedI applied the patch and had to reinstall the tour module (as you do).
Manually tested and looks good. The tour worked on the shortcut page.
Comment #15
r_morgan CreditAttribution: r_morgan commentedI got the patch to apply.
The tour ran fine, after reinstalling the tour module just like aczietlow said above.
I have one concern though, to a new Drupal user the last tour item that points to removing a link would probably seem to point to a space that does nothing. To people who have been around a while it make sense to click the drop down arrow first but it might confuse new people.
Comment #16
Ec1ipsis CreditAttribution: Ec1ipsis commentedI think that most internet users are fairly familiar with drop downs/expansion by now. Given the pervasiveness of select lists and expanding menus in mobile responsiveness, I think it should be fairly intuitive. Anyone else have thoughts on this?
Comment #17
aczietlow CreditAttribution: aczietlow commentedAttaching an screenshot that better captures the point made in #15
The issue is with step #5 in the tour. I think it would make sense to not have it overlap the drop down menu.
Comment #18
Ec1ipsis CreditAttribution: Ec1ipsis commentedAhh! I see. Agreed, I'll see about adjusting that and get a new patch in for testing.
Comment #19
Ec1ipsis CreditAttribution: Ec1ipsis commentedEssentially, the target in question is too small to be accurately targeted by the arrow. Joyride aligns itself in relationship to the element, and then positions the nub about 25 pixels off of the corner in question. In practice, if you use "bottom" for the location (this is the default), Joyride will place the tip below the item, with the left edge of the tip aligned to the left edge of the element. The nub will then be approximately 25px from the top left corner, but the element in question is only about half that size, so the arrow appears to point to empty space. This is the behavior observed by rmorgan and aczietlow, but there isn't a position argument that adequately causes the arrow to point to the element. Drupal doesn't support the nubPosition argument that Joyride does, but even adding this argument wouldn't be sufficient to solve the problem of very small elements due to the limitations in the Joyride library.
With this in mind, I added some extra text to tip four indicating that the dropdown arrow was also responsible for deletions, and removed tip five. This patch is attached. Thoughts?
EDIT: For greater clarity and great justice!! (Or maybe just the clarity.)
Comment #21
Ec1ipsis CreditAttribution: Ec1ipsis commentedUploaded working patch.
Comment #22
aczietlow CreditAttribution: aczietlow commentedMarking for testing
Comment #23
aczietlow CreditAttribution: aczietlow commented21: tour.shortcut-19.patch queued for re-testing.
Comment #24
aczietlow CreditAttribution: aczietlow commentedFor testing the patch, I applied the patch, uninstalled and reinstalled the shortcuts module, and rebuilt the cache. I then navigate to the shortcuts management page (admin/config/user-interface/shortcut/manage/default/customize)
I'm only seeing 2 of the 4 steps listed in the patch.
Comment #25
Wim LeersI can see all four tour tips. But I think the wording/copywritings needs some love — it feels very rough. It doesn't have to be perfect though.
This also needs test coverage, see #1 and
ViewsUITourTest
.Comment #26
webchickThis one seems like a good thing to do, but postponed on #1921152-109: META: Start providing tour tips for other core modules. for now.
Comment #27
mgiffordComment #33
mkindred CreditAttribution: mkindred commentedWorking on this issue from mentored core sprint @ Nashville DrupalCon.
Comment #34
mkindred CreditAttribution: mkindred commentedPatch for the first shortcuts management page. I'd think that subsequent pages are mostly common UI, so probably won't need tour elements.
Comment #35
mkindred CreditAttribution: mkindred commentedComment #36
Farnoosh CreditAttribution: Farnoosh as a volunteer commenteduuid needs to be removed. please add another patch. Thanks
Comment #37
Farnoosh CreditAttribution: Farnoosh as a volunteer commentedComment #38
mkindred CreditAttribution: mkindred commentedHa ha, you're correct. This should be better.
Comment #39
Farnoosh CreditAttribution: Farnoosh as a volunteer commentedRemove hyphenated word in the shortcut general, I guess. Thanks
Comment #40
mkindred CreditAttribution: mkindred commentedComment #43
mkindred CreditAttribution: mkindred commentedUpdated patch:
Comment #45
rakesh.gectcrComment #50
ridhimaabrol24 CreditAttribution: ridhimaabrol24 at Srijan | A Material+ Company for Drupal India Association commentedFixed failed tests and added a test for the Shortcut Tour tips.
Comment #52
ridhimaabrol24 CreditAttribution: ridhimaabrol24 at Srijan | A Material+ Company for Drupal India Association commentedFixing failed tests.
Comment #53
ridhimaabrol24 CreditAttribution: ridhimaabrol24 at Srijan | A Material+ Company for Drupal India Association commentedHi
I have made some changes to the patch #52. Replaced id and class with proper data-class.
Also I have removed the tip pointing to the main navigation menu because that is not the part of the Shortcuts page and it fails the tests as the corresponding element doesn't gets found on the shortcuts page.
Also added a tour tip to the "Add shortcuts" Link.
Comment #54
jibranThank you @ridhimaabrol24 for working on the patch.
This property should be protected now.
Comment #55
ridhimaabrol24 CreditAttribution: ridhimaabrol24 at Srijan | A Material+ Company for Drupal India Association commentedThank you for the feedback @jibran. Made the changes.
Comment #59
manuel.adanAccommodate later changes in tour configuration files. Simple diff provided, interdiff failed due changes in base code. Re-rolled.
Comment #61
manuel.adanFixes configuration entries order.
Comment #64
smustgrave CreditAttribution: smustgrave at Mobomo commentedSo this is now postponed as shortcut is going to be removed #3311180: [meta] Determine how to make Shortcut module more useful and maintainable
Also tour is update for consideration #3325445: [Policy] Remove tour module from core
But positive news if tour moves out it has less gates to go through in contrib.