I use Firefox 71.0 on Ubuntu 18.04 btw... just in case this changes something Created attachment 96899 [details] [review] Bug 24363: Small CSS change to align datepicker within input This is a small CSS change to align the datepicker within the input TEST PLAN: 1. Look at additem.pl datepicker and notice it isn't too sexy. 2. Apply patch and regenerate CSS (https://wiki.koha-community.org/wiki/Working_with_SCSS_in_the_OPAC_and_staff_client) 3. Look at the additem.pl datepicker and see that it looks better. 4. Since the SCSS for ui-datepicker-trigger is global take some time to look at other date pickers in the Koha staff client and confirm they also look good. in additem, I would center it also, so that it looks like the other ones. I tried to track downs all the other pages where the calendar icon appears (when the bullet is a star, the calendar is wonky): - memberentry - moremember (when adding a manual restriction) * request (when placing a hold and in the existing holds list) - holidays - lateorders * suggestion (when adding a new suggestion (in suggestion management section), when searching for a suggestion) - invoices - aqbudgetperiods * neworderempty (when adding an item while placing an order) - histsearch - parcels - orderreceive * circulation (when picking a due date or a renewal date) * returns (when specifying a return date) - pendingreserves - reserveratios - subscription-add - serials-edit (in the receiving table, but also in the item form if the issue has an item) - checkexpiration - subscription-numberpatterns - clubs-add-modify * import-borrowers * cleanborrowers * modborrowers - review * batchMod - export * inventory - viewlog - koha-news - scheduler - categories - smart-rules I may be missing some (or a lot). Created attachment 96968 [details] [review] Bug 24363: Vertical align datepicker icon on additem (In reply to Jonathan Druart from comment #4) > Created attachment 96968 [details] [review] [review] > Bug 24363: Vertical align datepicker icon on additem That was my try to fix the additem form, but it was not perfect as the '...' was not displaying good. On the hold list view: https://snipboard.io/iLdfAg.jpg I was encountering quite a bit of broken displays yesterday - feel we might need to revert this for hte moment. (In reply to Katrin Fischer from comment #7) > I was encountering quite a bit of broken displays yesterday - feel we might > need to revert this for hte moment. I do not think we should revert it, it only impacts master. Also the fix should be quite small (fix all issues with a few CSS lines). Just need to find the correct fix :) Created attachment 97466 [details] [review] Bug 24363: Datepicker calendar is not sexy on additem.pl This patch is essentially an alternative to Bug 24181. Instead of trying to make the datepicker inputs match default fields, it defines a default style for text inputs, selects, and textareas. This new default style borrows some from Bootstrap's input styling and some from the Select2 plugin's style. This makes forms like the one on the add item page have unified styling for all fields. To test, apply the patch and rebuild the staff client CSS (https://wiki.koha-community.org/wiki/Working_with_SCSS_in_the_OPAC_and_staff_client). Test as many pages as you can to look for consistent styling of form fields: Patron entry, cataloging (basic and advanced), reports, batch item modification, system preferences, etc. I see it wider than other inputs: https://snipboard.io/pemyLA.jpg Looks ok without padding-right: 18px; I'll add some more problem pages: * Reports: /cgi-bin/koha/reports/issues_stats.pl Everything looks good in Firefox. However with Google Chrome the icon is aligned to the inside left of the box and the start of the date overlays it. I can post screenshots if required. Created attachment 98289 [details] [review] Bug 24363: Do not use the two-value syntax for background-position-x It's not supported by chrome and chromium (inspector says "invalidd property value". See also: https://developer.mozilla.org/en-US/docs/Web/CSS/background-position-x section "Browser compatibility" (In reply to David Nind from comment #13) > Everything looks good in Firefox. > > However with Google Chrome the icon is aligned to the inside left of the box > and the start of the date overlays it. > > I can post screenshots if required. I confirm that on both Chrome and Chromium, inspector says "invalid property value" for rule background-position-x. The new patch should fix it. This now looks good to me! (Firefox + Google Chrome on Ubuntu 18.04) Should Jonathan's patch include the updated CSS file? Originally I had to regenerate the CSS. Created attachment 98355 [details] Bug 24363 - Browse button broken Actually, the patch "breaks" the 'Browse' button in Firefox. Displays fine in Google Chrome. See the attachment with before and after screenshots. It did this as well when originally tested. Created attachment 98366 [details] [review] Bug 24363: Do not use the two-value syntax for background-position-x It's not supported by chrome and chromium (inspector says "invalidd property value". See also: https://developer.mozilla.org/en-US/docs/Web/CSS/background-position-x section "Browser compatibility" (In reply to David Nind from comment #16) > Should Jonathan's patch include the updated CSS file? Originally I had to > regenerate the CSS. No, it should not. Resubmitted. (In reply to David Nind from comment #17) > Created attachment 98355 [details] > Bug 24363 - Browse button broken > > Actually, the patch "breaks" the 'Browse' button in Firefox. Displays fine > in Google Chrome. See the attachment with before and after screenshots. > > It did this as well when originally tested. Indeed, I recreate that. Apparently it is caused by: + height: 26px; Owen, could you explain where do come from the @mixin default-input rules? Do we really need them? > > Actually, the patch "breaks" the 'Browse' button in Firefox. The difference is almost imperceptible in Firefox on Windows, so I'm glad you caught it. > Owen, could you explain where do come from the @mixin default-input rules? > Do we really need them? We don't *need* them but using a @mixin lets us define the rules once and apply them to more than one element. I don't think the mixin is a problem if we add "height:unset" to input[type='file']. Created attachment 98387 [details] [review] Bug 24363: (follow-up) Remove height from input[type='file'] Created attachment 98426 [details] [review] Bug 24363: Datepicker calendar is not sexy on additem.pl This patch is essentially an alternative to Bug 24181. Instead of trying to make the datepicker inputs match default fields, it defines a default style for text inputs, selects, and textareas. This new default style borrows some from Bootstrap's input styling and some from the Select2 plugin's style. This makes forms like the one on the add item page have unified styling for all fields. To test, apply the patch and rebuild the staff client CSS (https://wiki.koha-community.org/wiki/Working_with_SCSS_in_the_OPAC_and_staff_client). Test as many pages as you can to look for consistent styling of form fields: Patron entry, cataloging (basic and advanced), reports, batch item modification, system preferences, etc. Signed-off-by: David Nind <david@davidnind.com> Created attachment 98427 [details] [review] Bug 24363: Do not use the two-value syntax for background-position-x It's not supported by chrome and chromium (inspector says "invalidd property value". See also: https://developer.mozilla.org/en-US/docs/Web/CSS/background-position-x section "Browser compatibility" Signed-off-by: David Nind <david@davidnind.com> Created attachment 98428 [details] [review] Bug 24363: (follow-up) Remove height from input[type='file'] Signed-off-by: David Nind <david@davidnind.com> Magic! Have tested in Google Chrome and Firefox Developer Edition on Ubuntu 18.04. Everything looks much better - date pickers and upload buttons. While I didn't check all the screens with data pickers, I checked a fair number of them and they all look consistent. I think there is still a problem with the inputs (radio now): Before: https://snipboard.io/plg4Ye.jpg After : https://snipboard.io/KHaCVI.jpg Alignment and padding/margin? Created attachment 98463 [details]
Image "diff" of the two screenshots
I don't think this difference warrants failing the patch.
I am worried they are not the only regressions we are going to introduce with this patch as it can impact all the inputs. Also notice that the input in the header has now a shadow. I think that if we cannot fix the regressions brought by bug 24181 easily, we will need to revert it. Created attachment 98857 [details] [review] Bug 24363: Move the datepicker icon out of the input (In reply to Jonathan Druart from comment #30) > Created attachment 98857 [details] [review] [review] > Bug 24363: Move the datepicker icon out of the input What about this patch? https://snipboard.io/Lud1yE.jpg https://snipboard.io/d47B2a.jpg https://snipboard.io/0YQydX.jpg Created attachment 98893 [details] [review] Bug 24363: Move the datepicker icon out of the input Signed-off-by: David Nind <david@davidnind.com> Created attachment 99139 [details] [review] Bug 24363: Move the datepicker icon out of the input Signed-off-by: David Nind <david@davidnind.com> Nice work everyone! Pushed to master for 20.05 enhancement not backported to 19.11.x |
Created attachment 96893 [details] Screenshot showing the date acquired field in additem I just wanted to mention that the datepicker calendar icon looks weird in the additem form (probably elsewhere too). Could we make it bigger and centered? Also the ellipsis (...) is not next to the box as it was before.