Bug 32028 - Add page-section to various administration pages (part 2)
Summary: Add page-section to various administration pages (part 2)
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Staff interface (show other bugs)
Version: master
Hardware: All All
: P5 - low minor (vote)
Assignee: Lucas Gass
QA Contact: Katrin Fischer
URL:
Keywords:
Depends on: 30952
Blocks: 31757
  Show dependency treegraph
 
Reported: 2022-10-27 21:02 UTC by Katrin Fischer
Modified: 2023-06-08 22:28 UTC (History)
4 users (show)

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


Attachments
Bug 32028: Add page-section to smart-rules.tt and fix markup errors (151.54 KB, patch)
2022-11-02 23:39 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 32028: Add page-section to various admin pages (56.00 KB, patch)
2022-11-03 17:50 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 32028: Add page-section to smart-rules.tt and fix markup errors (151.17 KB, patch)
2022-11-04 14:53 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 32028: Add page-section to various admin pages (56.03 KB, patch)
2022-11-04 17:46 UTC, David Nind
Details | Diff | Splinter Review
Bug 32028: Add page-section to smart-rules.tt and fix markup errors (151.22 KB, patch)
2022-11-04 17:46 UTC, David Nind
Details | Diff | Splinter Review
Bug 32028: Add page-section to various admin pages (56.14 KB, patch)
2022-11-09 12:42 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 32028: Add page-section to smart-rules.tt and fix markup errors (151.33 KB, patch)
2022-11-09 12:43 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 32028: (follow-up) Improve smart rules formatting (183.96 KB, patch)
2022-11-09 12:52 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 32028: (follow-up) Improve smart rules formatting (184.01 KB, patch)
2022-11-09 12:58 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 32028: Add page-section to various admin pages (56.14 KB, patch)
2022-11-09 13:18 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 32028: Add page-section to smart-rules.tt and fix markup errors (151.33 KB, patch)
2022-11-09 13:18 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 32028: (follow-up) Improve smart rules formatting (183.80 KB, patch)
2022-11-09 13:18 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 32028: (follow-up) Fix headings levels (4.40 KB, patch)
2022-11-09 13:25 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Katrin Fischer 2022-10-27 21:02:43 UTC
Bug 31986 is part 1, these could be part 2:

* Jobs 
* circulation rules
* Patron attributes
* Cities
* Patron restrictions 
* Debit types
* Credit types
* Cash registers
* Plugins
Comment 1 Lucas Gass 2022-11-02 23:39:21 UTC
Created attachment 143052 [details] [review]
Bug 32028: Add page-section to smart-rules.tt and fix markup errors

To test:
1. Apply patch
2. Go to Adminstration > Circulation and fine rules
3. Look at the page-section added and make sure each looks good.
4. Use an HTML validator to check and make sure the HTML looks good.

Note: There are indentation changes and some fixes to bad markup ( stray tags, etc ). Since there were quite a few mark-up errors I am attaching the smart-rules.tt patch by itself. The rest of the Adminstration pages mentioned will be in a single patch that is incoming!
Comment 2 Lucas Gass 2022-11-03 17:50:34 UTC
Created attachment 143146 [details] [review]
Bug 32028: Add page-section to various admin pages

This patch adds the page section to div to the following administration pages:

admin/cash_registers.tt
admin/cities.tt
admin/credit_types.tt
admin/debit_types.tt
admin/patron-attr-types.tt
admin/restrictions.tt
plugins/plugins-home.tt

To test go the following pages and make sure everything looks good:

Administration > Cash registers. You must enable the sys pref 'UseCashRegisters'
Administration > Cities and towns.
Administration > Credit types
Administration > Debit types
Administration > Patron restrictions
Adminstration > Patron attribute types
Administration > Manage plugins
Comment 3 Lucas Gass 2022-11-03 17:52:05 UTC
I believe the background jobs page has already handled in Bug 31960.
Comment 4 David Nind 2022-11-04 08:41:47 UTC
The patch no longer applies 8-(..

git bz apply 32028

Bug 32028 - Add page-section to various administration pages (part 2)

143052 - Bug 32028: Add page-section to smart-rules.tt and fix markup errors
143146 - Bug 32028: Add page-section to various admin pages

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 32028: Add page-section to smart-rules.tt and fix markup errors
Using index info to reconstruct a base tree...
M	koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt
Falling back to patching base and 3-way merge...
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt
CONFLICT (content): Merge conflict in koha-tmpl/intranet-tmpl/prog/en/modules/admin/smart-rules.tt
error: Failed to merge in the changes.
Patch failed at 0001 Bug 32028: Add page-section to smart-rules.tt and fix markup errors
Comment 5 Lucas Gass 2022-11-04 14:53:46 UTC
Created attachment 143214 [details] [review]
Bug 32028: Add page-section to smart-rules.tt and fix markup errors

To test:
1. Apply patch
2. Go to Adminstration > Circulation and fine rules
3. Look at the page-section added and make sure each looks good.
4. Use an HTML validator to check and make sure the HTML looks good.

Note: There are indentation changes and some fixes to bad markup ( stray tags, etc ).
Comment 6 David Nind 2022-11-04 17:46:08 UTC
Created attachment 143260 [details] [review]
Bug 32028: Add page-section to various admin pages

This patch adds the page section to div to the following administration pages:

admin/cash_registers.tt
admin/cities.tt
admin/credit_types.tt
admin/debit_types.tt
admin/patron-attr-types.tt
admin/restrictions.tt
plugins/plugins-home.tt

To test go the following pages and make sure everything looks good:

Administration > Cash registers. You must enable the sys pref 'UseCashRegisters'
Administration > Cities and towns.
Administration > Credit types
Administration > Debit types
Administration > Patron restrictions
Adminstration > Patron attribute types
Administration > Manage plugins

Signed-off-by: David Nind <david@davidnind.com>
Comment 7 David Nind 2022-11-04 17:46:13 UTC
Created attachment 143261 [details] [review]
Bug 32028: Add page-section to smart-rules.tt and fix markup errors

To test:
1. Apply patch
2. Go to Adminstration > Circulation and fine rules
3. Look at the page-section added and make sure each looks good.
4. Use an HTML validator to check and make sure the HTML looks good.

Note: There are indentation changes and some fixes to bad markup ( stray tags, etc ).

Signed-off-by: David Nind <david@davidnind.com>
Comment 8 David Nind 2022-11-04 17:53:52 UTC
Questions from testing:

1. Delete confirmation messages: there are two styles, assuming there will be bugs to fix these at some stage to make consistent:
   - Patron restrictions - "new" style (confirmation message in a page section div)
   - Patron attribute types - "old" yellow style (yellow box and asking you to confirm deletion)
   - Cities and towns - "old" yellow style

2. Patron restrictions: Delete - 'Delete this restriction' and cancel buttons are in the page section div.

3. Plugins: has the tools sidebar menu, breadcrumb, and page title - if there is not a bug for that already, then I can log one.

Other notes:

1. Plugins: installed three plugins - BiblbioCommons connector plugin for Koha, Example Kitchen-Sink Plugin, and SMS::Send::LinkMobility::Driver. Assuming it will be up to the plugin author to update any pages for their plugin, for example, the kitchen sink plugin configure section.

2. Circulation and fines rules: 
   - before patch applied any before any changes made: 0 errors, 73 warnings (using HTML Validator for Firefox - https://www.gueury.com/mozilla/)
   - after applying patch: 0 errors, 54 warnings
   - not sure of the "best" tool to do this or where the threshold is for addressing any issues
Comment 9 Katrin Fischer 2022-11-09 12:41:19 UTC
> 1. Delete confirmation messages: there are two styles, assuming there will
> be bugs to fix these at some stage to make consistent:
>    - Patron restrictions - "new" style (confirmation message in a page
> section div)
>    - Patron attribute types - "old" yellow style (yellow box and asking you
> to confirm deletion)
>    - Cities and towns - "old" yellow style

I think it would be nice to have a separate bug about these. We should tackle them at some point for sure, just a lot of things to go through right now.


> 2. Patron restrictions: Delete - 'Delete this restriction' and cancel
> buttons are in the page section div.

Bit confused about this one. Aren't they all in the table?


> 3. Plugins: has the tools sidebar menu, breadcrumb, and page title - if
> there is not a bug for that already, then I can log one.

I believe that's because you can also access some plugins from the tools page. Could you file separately please?


> Other notes:
> 
> 1. Plugins: installed three plugins - BiblbioCommons connector plugin for
> Koha, Example Kitchen-Sink Plugin, and SMS::Send::LinkMobility::Driver.
> Assuming it will be up to the plugin author to update any pages for their
> plugin, for example, the kitchen sink plugin configure section.
> 
> 2. Circulation and fines rules: 
>    - before patch applied any before any changes made: 0 errors, 73 warnings
> (using HTML Validator for Firefox - https://www.gueury.com/mozilla/)
>    - after applying patch: 0 errors, 54 warnings
>    - not sure of the "best" tool to do this or where the threshold is for
> addressing any issues

On this page the h2 headings are outside the page-sections, but for the advanced search and on other pages they are inside. I think this is already a good change, but we really need to write our style guide about these kinds of things.
Comment 10 Katrin Fischer 2022-11-09 12:42:54 UTC
Created attachment 143498 [details] [review]
Bug 32028: Add page-section to various admin pages

This patch adds the page section to div to the following administration pages:

admin/cash_registers.tt
admin/cities.tt
admin/credit_types.tt
admin/debit_types.tt
admin/patron-attr-types.tt
admin/restrictions.tt
plugins/plugins-home.tt

To test go the following pages and make sure everything looks good:

Administration > Cash registers. You must enable the sys pref 'UseCashRegisters'
Administration > Cities and towns.
Administration > Credit types
Administration > Debit types
Administration > Patron restrictions
Adminstration > Patron attribute types
Administration > Manage plugins

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

https://bugs.koha-community.org/show_bug.cgi?id=32082
Comment 11 Katrin Fischer 2022-11-09 12:43:06 UTC
Created attachment 143499 [details] [review]
Bug 32028: Add page-section to smart-rules.tt and fix markup errors

To test:
1. Apply patch
2. Go to Adminstration > Circulation and fine rules
3. Look at the page-section added and make sure each looks good.
4. Use an HTML validator to check and make sure the HTML looks good.

Note: There are indentation changes and some fixes to bad markup ( stray tags, etc ).

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

https://bugs.koha-community.org/show_bug.cgi?id=32082
Comment 12 Martin Renvoize 2022-11-09 12:52:46 UTC Comment hidden (obsolete)
Comment 13 Martin Renvoize 2022-11-09 12:58:43 UTC
Created attachment 143503 [details] [review]
Bug 32028: (follow-up) Improve smart rules formatting

I correct the indenting here, so remember to diff ignoring whitespace ;)

I remove some of the page-section divs introduced by the previous patch
and instead replace the 'container' classes with the new 'page-section
class on existing divs.

This brings the page-sections more in line with elsewhere in the
codebase.

I also update the 'h3' headings to 'h2' as we were somehow skipping that
heading level page wide and update the final 'h4' heading to 'h2' to
match the other section headings appropriately.

Finally, I break up the top section more, using bg-info for the top
block of instructions.
Comment 14 Martin Renvoize 2022-11-09 12:59:58 UTC
Katrin and I clashed.. just as I was assigning it to myself for QA she passed it ;).

Please check my final followup Katrin.. I think it improves the smart rules a fair bit ;P
Comment 15 Tomás Cohen Arazi 2022-11-09 13:07:13 UTC
Pushed to master for 22.11.

Nice work everyone, thanks!
Comment 16 Katrin Fischer 2022-11-09 13:18:31 UTC
Created attachment 143506 [details] [review]
Bug 32028: Add page-section to various admin pages

This patch adds the page section to div to the following administration pages:

admin/cash_registers.tt
admin/cities.tt
admin/credit_types.tt
admin/debit_types.tt
admin/patron-attr-types.tt
admin/restrictions.tt
plugins/plugins-home.tt

To test go the following pages and make sure everything looks good:

Administration > Cash registers. You must enable the sys pref 'UseCashRegisters'
Administration > Cities and towns.
Administration > Credit types
Administration > Debit types
Administration > Patron restrictions
Adminstration > Patron attribute types
Administration > Manage plugins

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

https://bugs.koha-community.org/show_bug.cgi?id=32082
Comment 17 Katrin Fischer 2022-11-09 13:18:38 UTC
Created attachment 143507 [details] [review]
Bug 32028: Add page-section to smart-rules.tt and fix markup errors

To test:
1. Apply patch
2. Go to Adminstration > Circulation and fine rules
3. Look at the page-section added and make sure each looks good.
4. Use an HTML validator to check and make sure the HTML looks good.

Note: There are indentation changes and some fixes to bad markup ( stray tags, etc ).

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

https://bugs.koha-community.org/show_bug.cgi?id=32082
Comment 18 Katrin Fischer 2022-11-09 13:18:46 UTC
Created attachment 143508 [details] [review]
Bug 32028: (follow-up) Improve smart rules formatting

I correct the indenting here, so remember to diff ignoring whitespace ;)

I remove some of the page-section divs introduced by the previous patch
and instead replace the 'container' classes with the new 'page-section
class on existing divs.

This brings the page-sections more in line with elsewhere in the
codebase.

I also update the 'h3' headings to 'h2' as we were somehow skipping that
heading level page wide and update the final 'h4' heading to 'h2' to
match the other section headings appropriately.

Finally, I break up the top section more, using bg-info for the top
block of instructions.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 19 Martin Renvoize 2022-11-09 13:25:05 UTC
Created attachment 143509 [details] [review]
Bug 32028: (follow-up) Fix headings levels

This patch corrects the headings levels on cash_registers,
patron-attr-types and restrictions admin pages.

We also move the h2 headings inside the page-section div as appropriate.
Comment 20 Lucas Gass 2022-11-09 15:44:17 UTC
Cait and Martin, thanks for the follow-ups!