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.Problem/Motivation
The Add new page button on the list of Search pages admin/config/search/pages is inconsistent with other such buttons and vague. On other pages the usual format is Add foo: without the "new", and as a primary button. "Page" is also vague and could refer to any other type of page.
Proposed resolution
The button should be reworded as Add search page and then marked as a primary button.
Remaining tasks
User interface changes
This is a UI text change, so the hook_help text might need to edited as well.
API changes
Data model changes
| Comment | File | Size | Author |
|---|---|---|---|
| #17 | 2752915-17.patch | 3.63 KB | davic |
| #15 | Screen Shot 2016-06-27 at 10.15.53 AM.png | 61.66 KB | webchick |
| #15 | Screen Shot 2016-06-27 at 10.15.32 AM.png | 83.22 KB | webchick |
| #12 | save-search-page.png | 3.51 KB | ifrik |
| #12 | add-search-page.png | 14 KB | ifrik |
Comments
Comment #2
HazaHere is a small patch that changes the link name.
Tests have been updated too.
We were already using the "new search page" in the hook_help as far as I can see.
Comment #3
ifrikSorry,
I didn't notice that we also need to change another button for this to work consistently.
Once the user choose "Add search page" they get the usual form, and the a button that also says "Add search page". This should be then changed to "Save", similar to the buttons of other modules.
And any idea what triggers Bartik to add the "+" before as part of "Add foo" for other modules?
Comment #4
pashupathi nath gajawada CreditAttribution: pashupathi nath gajawada as a volunteer and at Melity commentedHi @ifrik,
I have done the changes as suggested in #3.
Please find the patch which would fix this .
Thanks,
Comment #6
HazaI think the bot is failling because of https://www.drupal.org/node/2572293#comment-11318935 (again, a PHP-FPM error). I can't ever tell how those tests are not failling on the other issues.
Comment #8
mohit_aghera CreditAttribution: mohit_aghera as a volunteer and at Axelerant commentedChanging few titles in test file.
Comment #10
ifrikHi pashupathi nath gajawada,
Can you in future please provide an interdiff when you make a new version of patch? That way we can see what the differences are between one version and the next. In this case it would help us to see whether the patch fails due to something that was changed in your patch, or whether it fails for some other reason.
Also: please don't change the patch name if you build on somebody else's work. Otherwise it looks like for no apparent reason you started again from scratch. It's basic good practice and respect to other contributors.
Comment #11
chr.fritschStart fixing the tests
Comment #12
ifrikThanks,
the wording on both buttons is now changed to "Add search page" and "Save".
I will post a separate issue for the UX problem that a page type needs to be selected before adding a search page, but that problem exists independent of the wording of the button.
Comment #13
ifrikComment #14
webchickWould be great to get some more complete before/after screenshots here, to help visualize the changes better. Also, I feel like this is part of a larger pattern, it'd be great to see the usability topic maintainers sign off on this. (If this already happened, a pointer to the issue comment is fine.)
Comment #15
webchickLooked into this a bit more... here's the "before" screenshots btw, since I was there anyway.
The two textual changes actually seem fine; "Add new page" is redundant, so "Add search page" seems better, and calling the save button on a single-purpose form "Save" is consistent with the rest of core. However, the change of the button to "primary" seems a bit controversial. First, because I'm not sure adding search pages is qualified as the "Primary" action on this page; there are many different field sets on this form. Secondly. because this creates two primary-styled buttons in very close proximity to one another, and we already have a big issue going on at #2520232: Separate the menu settings from the 'add link' button for a related problem with menu/taxonomy.
So I think if we re-roll this patch without the #primary change, the rest of this is probably good to go. And then, per xim, we should have a specific follow-up for changing the colour of the "add" button, along with a parent meta issue that describes the overall problem space of forms that allow you to add a thing and re-arrange a thing you just added on the same page. (The same issue would also be the parent of #2520232: Separate the menu settings from the 'add link' button).
Comment #16
webchickComment #17
davic CreditAttribution: davic at CI&T commentedFollowing #15 indications, it really seems that the primary type does not fit as well as we first tought for the button. This patch was made with all the name changes, but without the button type being primary.
Comment #18
davic CreditAttribution: davic at CI&T commentedComment #19
webchickComment #20
j2r CreditAttribution: j2r at Srijan | A Material+ Company commentedComment #21
j2r CreditAttribution: j2r at Srijan | A Material+ Company commentedLast patch #17 is working as expected
On Search page the button text is : "Add search page"
and on "Add new search page" button text is : "Save"
Comment #22
Wim LeersThis is technically a BC break, but it seems low-risk.
Comment #24
Wim LeersA fail after two weeks of passing all tests, that's suspicious. Retesting.
Comment #25
webchickAssuming this was meant instead.
Comment #27
davic CreditAttribution: davic at CI&T commentedIt feels like the fail is somewhat conditional of the test, as it fails suddenly. Rerolling to check again.
Comment #29
xjm@davic, thanks for updating the patch! (In the future, please provide an interdiff when you update patches to help reviewers.)
@Wim Leers:
Per our BC policy, strings and user interface elements are considered "internal" API and may change in minor releases:
https://www.drupal.org/core/d8-bc-policy#strings
https://www.drupal.org/core/d8-bc-policy#render-array
https://www.drupal.org/core/d8-bc-policy#themes
So we are good on that front for 8.2.x.
The updated patch is now straightforward: It standardizes the final button text to "Save" (which is consistent with what's used elsewhere throughout core), and makes the confusing "Add new page" button text specific so users know what it does. Based on that, I think we have sufficient review and signoffs to commit the text improvements.
Committed 978b026 and pushed to 8.2.x. Thanks!
To further improve the parent form, including the design and button coloration/styling for "Add a thing" on a page of "Configure the parent thing", let's continue the discussion in a new issue that draws on the discussion here, in #2520232: Separate the menu settings from the 'add link' button, and in the other issues mentioned there.
Comment #30
Gábor HojtsyThanks, removing from UX sprint now.