Bug 34412 - Inconsistencies in system preferences page titles, breadcrumbs, and header
Summary: Inconsistencies in system preferences page titles, breadcrumbs, and header
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Templates (show other bugs)
Version: Main
Hardware: All All
: P5 - low trivial (vote)
Assignee: Owen Leonard
QA Contact: Katrin Fischer
URL:
Keywords:
Depends on:
Blocks: 34396 35379
  Show dependency treegraph
 
Reported: 2023-07-25 22:04 UTC by Caroline Cyr La Rose
Modified: 2024-01-12 10:46 UTC (History)
1 user (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.11.00


Attachments
Bug 34412: Inconsistencies in System preferences page titles, breadcrumbs, and header (11.14 KB, patch)
2023-09-05 18:25 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 34412: Inconsistencies in System preferences page titles, breadcrumbs, and header (11.19 KB, patch)
2023-09-06 02:46 UTC, David Cook
Details | Diff | Splinter Review
Bug 34412: (follow-up) Clarify page heading, remove unused markup (2.96 KB, patch)
2023-09-06 11:00 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 34412: (QA follow-up) Terminology updates (5.05 KB, patch)
2023-09-08 10:50 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 34412: Inconsistencies in System preferences page titles, breadcrumbs, and header (11.19 KB, patch)
2023-09-08 11:15 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 34412: (follow-up) Clarify page heading, remove unused markup (3.01 KB, patch)
2023-09-08 11:15 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 34412: (QA follow-up) Terminology updates (5.11 KB, patch)
2023-09-08 11:15 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Caroline Cyr La Rose 2023-07-25 22:04:15 UTC
While testing bug 33909, I had to look at all the pages in the administration module. In doing so, I found some inconsistencies between the various pages. 

For the System preferences (local use preferences) page, here is what should be changed to make it consistent with the other pages in the module:

Page title
- Change 'Add a system preference' to 'New system preference'
- Change "Confirm deletion of parameter 'variable'" to "Confirm deletion of system preference 'variable'"
- Change 'Parameter deleted' to 'System preference deleted'

Breadcrumb
- Change 'Add a system preference' to 'New system preference'
- Change "Confirm deletion of parameter 'variable'" to "Confirm deletion of system preference 'variable'"
- In Confirm deletion, remove variable level
- Change 'Parameter deleted' to 'System preference deleted'

Page header
- Change 'Add a system preference' to 'New system preference'
- Styling of 'Add a system preference' header different than usual
- Change 'Modify a system preference' to "Modify system preference 'variable'"
- Styling of 'Modify a system preference' header different than usual
- Change "Confirm deletion of 'variable'?" to "Confirm deletion of system preference 'variable'"
- Change 'Data deleted' to 'System preference deleted'


Styling of the Confirm deletion page is different than usual. It's usually a warning (yellow background) centered on the page. The buttons are white with a green checkmark for 'Yes, delete' and white with a red cross for 'No, do not delete'
Comment 1 Owen Leonard 2023-09-05 18:25:06 UTC Comment hidden (obsolete)
Comment 2 David Cook 2023-09-06 02:46:12 UTC
Comment on attachment 155233 [details] [review]
Bug 34412: Inconsistencies in System preferences page titles, breadcrumbs, and header

Review of attachment 155233 [details] [review]:
-----------------------------------------------------------------

I have a few comments but none of them are blockers imo

::: koha-tmpl/intranet-tmpl/prog/en/modules/admin/systempreferences.tt
@@ +13,4 @@
>          [% END %] ›
>      [% END %]
>      [% IF ( add_validate ) %]
> +        [% t("Preference added") | html %] ›

I couldn't find a way to see this text but I'm not too worried about it.

@@ +91,5 @@
>  
>  [% IF ( add_form ) %]
> +        <h1>
> +            [% IF ( modify ) %]
> +                Modify a local use system preference

I think this could probably be "Modify preference '[% searchfield | html %]'", but not a deal breaker.

@@ +283,5 @@
>      [% END %]
>      
>      [% IF ( delete_confirm ) %]
> +        <div class="dialog alert">
> +            <h1>Confirm deletion of [% searchfield | html %]?</h1>

It would be a good idea to wrap [% searchfield | html %] in single quotes, but since this is the same before and after the change I don't think it's in scope.
Comment 3 David Cook 2023-09-06 02:46:43 UTC
Created attachment 155250 [details] [review]
Bug 34412: Inconsistencies in System preferences page titles, breadcrumbs, and header

This patch makes changes to the markup of various areas of the local use
system preferences template in order to make it more consistent with
other pages in Koha.

To test, apply the patch and go to Administration -> System
preferences -> Local use. View each variation of the page to confirm
that breadcrumb navigation, page title, and page headings are consistent
with each other.

- On the main page confirm that the heading is "Local use system
  preferences"
  - New preference:
    - The form on this page has been updated to make the markup
      structure more consistent with other pages, with the heading
      outside the white box of the form.
    - The two portions of the form have been combined into one so that
      the layout of the form is more consistent with similar forms.
 - Edit preference
 - Delete:
   - The deletion confirmation page has been reformatted to use an
     "alert" style dialog with styled confirmation buttons.
   - The "Data deleted" view has been reformatted to use a "message"
     style dialog.

Signed-off-by: David Cook <dcook@prosentient.com.au>
Comment 4 Owen Leonard 2023-09-06 11:00:59 UTC
Created attachment 155262 [details] [review]
Bug 34412: (follow-up) Clarify page heading, remove unused markup

This patch updates the heading for the "modify" view so that it includes
the name of the preference being modified.

The patch also removes markup related to the "add_validate"
state--a view confirming that a preference has been added--because that
view doesn't exist.
Comment 5 Katrin Fischer 2023-09-08 09:51:12 UTC
I feel it would be nicer to make some changes here:

* We agreed a while not to use "preference" or "sys pref" to always give the full context (consistency). So I think we should use "New system preference" etc.

* Instead of naming the placeholder searchfield, we should also name it {system_preference} because that is what will show up for translators and searchfield would likely cause confusion.

[% tx("Confirm deletion of preference '{searchfield}'", { searchfield = searchfield }) | html %] &rsaquo;
Comment 6 Owen Leonard 2023-09-08 10:50:42 UTC
Created attachment 155393 [details] [review]
Bug 34412: (QA follow-up) Terminology updates

- Use "system preference" instead of just "preference"
- Use "system_preference" as placeholder in translatable strings

This patch also updates a few places where the placeholder i18n syntax
can be used for consistency.
Comment 7 Katrin Fischer 2023-09-08 11:15:48 UTC
Created attachment 155401 [details] [review]
Bug 34412: Inconsistencies in System preferences page titles, breadcrumbs, and header

This patch makes changes to the markup of various areas of the local use
system preferences template in order to make it more consistent with
other pages in Koha.

To test, apply the patch and go to Administration -> System
preferences -> Local use. View each variation of the page to confirm
that breadcrumb navigation, page title, and page headings are consistent
with each other.

- On the main page confirm that the heading is "Local use system
  preferences"
  - New preference:
    - The form on this page has been updated to make the markup
      structure more consistent with other pages, with the heading
      outside the white box of the form.
    - The two portions of the form have been combined into one so that
      the layout of the form is more consistent with similar forms.
 - Edit preference
 - Delete:
   - The deletion confirmation page has been reformatted to use an
     "alert" style dialog with styled confirmation buttons.
   - The "Data deleted" view has been reformatted to use a "message"
     style dialog.

Signed-off-by: David Cook <dcook@prosentient.com.au>
Comment 8 Katrin Fischer 2023-09-08 11:15:51 UTC
Created attachment 155402 [details] [review]
Bug 34412: (follow-up) Clarify page heading, remove unused markup

This patch updates the heading for the "modify" view so that it includes
the name of the preference being modified.

The patch also removes markup related to the "add_validate"
state--a view confirming that a preference has been added--because that
view doesn't exist.

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 9 Katrin Fischer 2023-09-08 11:15:53 UTC
Created attachment 155403 [details] [review]
Bug 34412: (QA follow-up) Terminology updates

- Use "system preference" instead of just "preference"
- Use "system_preference" as placeholder in translatable strings

This patch also updates a few places where the placeholder i18n syntax
can be used for consistency.

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 10 Tomás Cohen Arazi 2023-09-08 14:54:27 UTC
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 11 Fridolin Somers 2023-09-08 19:57:21 UTC
Like others, not backported to 23.05.x