Bug 37274 - Standardize the toolbar in Vue components
Summary: Standardize the toolbar in Vue components
Status: Pushed to main
Alias: None
Product: Koha
Classification: Unclassified
Component: ERM (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low enhancement
Assignee: Jonathan Druart
QA Contact: Martin Renvoize (ashimema)
URL:
Keywords: Academy
Depends on:
Blocks: 37301 37929
  Show dependency treegraph
 
Reported: 2024-07-08 16:57 UTC by Esther Melander
Modified: 2024-09-16 11:04 UTC (History)
4 users (show)

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


Attachments
License header example (6.71 KB, image/png)
2024-07-08 16:57 UTC, Esther Melander
Details
Bug 37274: Use the Toolbar component in all Vue components (20.41 KB, patch)
2024-07-09 12:40 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 37274: Use the Toolbar component in all Vue components (20.44 KB, patch)
2024-07-10 10:09 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 37274: Use the Toolbar component in all Vue components (20.53 KB, patch)
2024-07-16 06:34 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Esther Melander 2024-07-08 16:57:38 UTC
Created attachment 168609 [details]
License header example

I wonder if the title of a License or Agreement should be changed from License #1 to the License name?

Example:
License #1 -> Ancestry

The reference to the license ID number does not hold a lot of significance. If a library has a lot of licenses/agreements, then the ID numbers are less useful. If a library did have multiple agreements/licenses with a particular vendor the license name could make the distinction instead.

The two icons for edit and delete could be a bit bigger with some space in-between. My first instinct was to look for the words Edit or Delete rather than an icon. Perhaps creating an Action drop down button would be better and Edit, Close, and Delete could be listed under one button.
Comment 1 Pedro Amorim 2024-07-09 08:57:38 UTC
I'd like to add here that we have a good opportunity to improve consistency in VueJS apps here going forward with this.
If you do some testing with the preservation module, (enable preservation syspref, and then create some processings at http://localhost:8081/cgi-bin/koha/preservation/settings) and you then create a new Train, and edit that same record, you'll notice there is a top toolbar with options like 'edit', 'delete', etc.

The above differs from what is currently happening in ERM, and I believe answers Esther's original comment.

We have a standard toolbar component added in bug 33417 that is being utilized by TrainsShow.vue but not by AgreementsShow.vue, and imo it should.
Comment 2 Jonathan Druart 2024-07-09 12:40:32 UTC
Created attachment 168646 [details] [review]
Bug 37274: Use the Toolbar component in all Vue components

We have a Toolbar component that must be used everywhere for
consistency.
This patch adds it when missing:
* ERM/AgreementsShow
Show an agreement

* ERM/EHoldingsLocalPackagesShow
Show a local package
* ERM/EHoldingsLocalTitlesShow
Show a local title

* ERM/LicensesShow
Show a license

* ERM/UsageStatisticsDataProvidersShow
Show a data provider

* Preservation/SettingsProcessingsShow
Show the the settings of a processing (Preservation module)

Confirm that the "Edit" and "Delete" icons are now replaced with buttons
(in a toolbar) on those views.
Comment 3 Pedro Amorim 2024-07-10 10:09:12 UTC
Created attachment 168706 [details] [review]
Bug 37274: Use the Toolbar component in all Vue components

We have a Toolbar component that must be used everywhere for
consistency.
This patch adds it when missing:
* ERM/AgreementsShow
Show an agreement

* ERM/EHoldingsLocalPackagesShow
Show a local package
* ERM/EHoldingsLocalTitlesShow
Show a local title

* ERM/LicensesShow
Show a license

* ERM/UsageStatisticsDataProvidersShow
Show a data provider

* Preservation/SettingsProcessingsShow
Show the the settings of a processing (Preservation module)

Confirm that the "Edit" and "Delete" icons are now replaced with buttons
(in a toolbar) on those views.

Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Comment 4 Katrin Fischer 2024-07-11 08:05:07 UTC
Could we simplify the bug description a bit?
Comment 5 Martin Renvoize (ashimema) 2024-07-16 06:34:08 UTC
Created attachment 169041 [details] [review]
Bug 37274: Use the Toolbar component in all Vue components

We have a Toolbar component that must be used everywhere for
consistency.
This patch adds it when missing:
* ERM/AgreementsShow
Show an agreement

* ERM/EHoldingsLocalPackagesShow
Show a local package
* ERM/EHoldingsLocalTitlesShow
Show a local title

* ERM/LicensesShow
Show a license

* ERM/UsageStatisticsDataProvidersShow
Show a data provider

* Preservation/SettingsProcessingsShow
Show the the settings of a processing (Preservation module)

Confirm that the "Edit" and "Delete" icons are now replaced with buttons
(in a toolbar) on those views.

Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 6 Martin Renvoize (ashimema) 2024-07-16 06:35:19 UTC
This is a good consistency improvement.. but the code is rather wet... I'm really looking forward to the work on bug 37301 to DRY is out a lot and made this a more re-usable piece.

Passing QA on this one though to keep the ball rolling.
Comment 7 Katrin Fischer 2024-07-18 16:27:33 UTC
Pushed for 24.11!

Well done everyone, thank you!