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.
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.
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.
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>
Could we simplify the bug description a bit?
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>
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.
Pushed for 24.11! Well done everyone, thank you!