Created attachment 149453 [details] screenshot of UI
* There's a problem with the "filter by mine" filter at the top * There's a problem with the table not going full width, this is not as trivial as the fix I did for bug 33066 here https://bugs.koha-community.org/bugzilla3/page.cgi?id=splinter.html&bug=33066&attachment=149183 Can't just move the table out of the <fieldset> since it's a nested component.
Created attachment 149474 [details] [review] Bug 33491: Fix UI issues in EHoldingsEBSCOPackageAgreements
This patch is not ideal but it's fixing the main problems. Pedro, do you think it's acceptable as it, at least for backport maybe then we see how we can improve this view later?
It doesn't fix the "show mine only" being punted to the left, this is because it's getting a float:left also from being a child of <fieldset> (which is also causing the table to not be full width). The problem is <EHoldingsPackageAgreements> sits inside a <fieldset> in EHoldingsEBSCOPackagesShow.vue. Then, in EHoldingsEBSCOPackageAgreements.vue we're opening the modal that has <AgreementsList> inside it, but that modal is already a child of the aforementioned <fieldset> so it's inheriting its style unintendendly. There is also another problem: the modal needs a overflow (and perhaps a max-height?). If we have many agreements created, the modal overflows the screen and doesn't let the user scroll vertically.
Created attachment 149778 [details] [review] Bug 33491: Fix UI issues in EHoldingsEBSCOPackageAgreements
(In reply to Pedro Amorim from comment #4) > It doesn't fix the "show mine only" being punted to the left, this is > because it's getting a float:left also from being a child of <fieldset> > (which is also causing the table to not be full width). > > The problem is <EHoldingsPackageAgreements> sits inside a <fieldset> in > EHoldingsEBSCOPackagesShow.vue. > > Then, in EHoldingsEBSCOPackageAgreements.vue we're opening the modal that > has <AgreementsList> inside it, but that modal is already a child of the > aforementioned <fieldset> so it's inheriting its style unintendendly. Fixed in the last version of the patch. > There is also another problem: the modal needs a overflow (and perhaps a > max-height?). If we have many agreements created, the modal overflows the > screen and doesn't let the user scroll vertically. I would suggest to deal with that on a separate bug. I think we need a separate component for modal, in which we will deal with this issue (we use .modal in a couple of places already)
Created attachment 149823 [details] [review] Bug 33491: Fix UI issues in EHoldingsEBSCOPackageAgreements Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Created attachment 149853 [details] [review] Bug 33491: Fix UI issues in EHoldingsEBSCOPackageAgreements Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
This is certainly an improvement, Passing QA.
Pushed to master for 23.05. Nice work everyone, thanks!
Created attachment 149924 [details] comparison Top picture is the header in agreements list before the patch, bottom picture is after the patch. This CSS in this patch is being applied in agreements list unintentionally. .filters input[type="checkbox"] { margin-left: 0 !important; }
Created attachment 150505 [details] [review] Bug 33491: Do not apply style to agreement list view We removed the 'scoped' attribute and so the style rules added in the previous patch were applied to the agreement list view. Why is 'scoped' not working is the main question here (?) but adding a more specific selector to aim only the component (AgreementsList) from the modal is a quick solution.
(In reply to Pedro Amorim from comment #11) > Created attachment 149924 [details] > comparison > > Top picture is the header in agreements list before the patch, bottom > picture is after the patch. > > This CSS in this patch is being applied in agreements list unintentionally. > > .filters input[type="checkbox"] { > margin-left: 0 !important; > } This patch is fixing the problem. Do you have something better to suggest?
Created attachment 150518 [details] [review] Bug 33491: Do not apply style to agreement list view We removed the 'scoped' attribute and so the style rules added in the previous patch were applied to the agreement list view. Why is 'scoped' not working is the main question here (?) but adding a more specific selector to aim only the component (AgreementsList) from the modal is a quick solution. Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com>
> Why is 'scoped' not working is the main question here As Pedro pointed to me, it is actually working as expected: style rules are defined in EHoldingsEBSCOPackageAgreements and the table is in AgreementList.
There is another problem: https://snipboard.io/Odhrai.jpg The .modal rule should apply to the modal of the EHoldingsEBSCOPackageAgreements component only.
Created attachment 150564 [details] [review] Bug 33491: Add a more specific class for the modal We don't want to apply these CSS rules to other modals
Nice work, thanks everyone! Pushed to 22.11.x for the next release.
PLEASE IGNORE THE ABOVE, NOT PUSHED TO STABLE
(In reply to Jonathan Druart from comment #17) > Created attachment 150564 [details] [review] [review] > Bug 33491: Add a more specific class for the modal > > We don't want to apply these CSS rules to other modals Pedro, should we go with this patch and see if we can do better on a separate bug?
Many hands makes light work, thankyou everyone! Pushed to 22.11.x for the next release
Missing dependencies for 22.05.x, no backport
Last patch still missing.
(In reply to Jonathan Druart from comment #23) > Last patch still missing. Sorry, I merged the last two patches. Don't remember how/why.
*** Bug 35132 has been marked as a duplicate of this bug. ***