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.Panopoly Magic redoes the vanilla Add Content dialog that's provided by the CTools/Panels/IPE. Instead of having the widget label be a link, Magic creates an "Add" link and styles it to look like a button. The Add link doesn't have any additional text to say what the link does out of context. This makes it impossible for a screenreader user to know what they'll add when they click the link. It also makes it harder to do automated testing of different widget types. If you use a When I click "Add" step, you'll only ever be able to click the first instance.
Adding invisible text including the widget label should fix both problems.
| Comment | File | Size | Author |
|---|---|---|---|
| #9 | interdiff.txt | 1.01 KB | dsnopek |
| #9 | panopoly_test-add-content-links-2773683-9.patch | 1.99 KB | dsnopek |
| #5 | panopoly-magic-add-content-links-2773683-5.patch | 753 bytes | cboyden |
Comments
Comment #2
cboyden CreditAttribution: cboyden commentedAdding invisible text is easy enough. However, making this change will the break button styling in Radix or in any theme that uses a similar approach in hook_preprocess_panels_add_content_link. The function depends on the title exactly matching
t('Add').Comment #3
cboyden CreditAttribution: cboyden commentedHere's a patch that adds invisible text to the Add link. See related issue #2773717: Theming for Add Content links depends on exact title text match in Radix for work on a fix for button styling.
Comment #4
dsnopekThe Panopoly code looks fine!
However, considering the Radix issue (and my comment there) it might make sense to also stash some additional information on the
$content_typearray, for example, (a) flag to indicate that this is an add button, or (b) the original title text so that we can reconstruct the translated title on the Radix end (but that's really just to determine if this is an add button, so doing (a) is cleaner).It'd also be really great to have a Behat test that takes advantage of this - there might be an existing Behat test that's doing something wonky to work around not being able to target a specific 'Add' button that we can just modify, or if not, create a new test scenario with the others that test the previews in the "Add content" modal.
Comment #5
cboyden CreditAttribution: cboyden commentedThanks, I've updated the patch to add a flag to the array that can then be used by themes to know that this is a panopoly_magic Add button.
There are no tests with an
I click "Add"step. I assume that means there are as yet no Behat tests that use the IPE to add any type of content other than the top-level widgets provided by Panopoly itself.Comment #6
dsnopekThe code looks great! This is just waiting on a Behat test, and then I think it's ready to commit. :-)
Comment #7
cboyden CreditAttribution: cboyden commentedHere's a patch for panopoly_test that adds a scenario to check that the link text in the Add button contains the widget title.
Comment #8
dsnopekThanks! Started a build on Travis:
https://travis-ci.org/panopoly/panopoly/builds/148192191
Comment #9
dsnopekAn old test that looked at (but didn't click) the "Add" buttons failed - here's an updated panopoly_test patch to fix it!
EDIT: Here's the Travis build: https://travis-ci.org/panopoly/panopoly/builds/148287721
Comment #11
dsnopekTests passed, so, committed! Thanks!