Summary: | Fix style of messages on Elasticsearch configuration page | ||
---|---|---|---|
Product: | Koha | Reporter: | Owen Leonard <oleonard> |
Component: | Templates | Assignee: | Owen Leonard <oleonard> |
Status: | CLOSED FIXED | QA Contact: | Testopia <testopia> |
Severity: | minor | ||
Priority: | P5 - low | CC: | andrew, fridolin.somers, jonathan.druart, martin.renvoize, victor |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
Change sponsored?: | --- | Patch complexity: | Small patch |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: |
21.05.00,20.11.06,20.05.12
|
|
Circulation function: | |||
Attachments: |
Bug 27695: Fix style of messages on Elasticsearch configuration page
Bug 27695: Fix style of messages on Elasticsearch configuration page Bug 27695: Fix style of messages on Elasticsearch configuration page Bug 27695: (follow-up) Add specificity to CSS changes Bug 27695: Fix style of messages on Elasticsearch configuration page Bug 27695: (follow-up) Add specificity to CSS changes |
Description
Owen Leonard
2021-02-12 17:07:40 UTC
Created attachment 116842 [details] [review] Bug 27695: Fix style of messages on Elasticsearch configuration page This patch makes minor modifications to SCSS and HTML to make messages on the Elasticsearch configuration page more consistent with other interfaces. To test you should have Elasticsearch selected as your search engine. - Apply the patch and rebuild the staff interface CSS (https://wiki.koha-community.org/wiki/Working_with_SCSS_in_the_OPAC_and_staff_client) - Go to Administration -> Search engine configuration (Elasticsearch) - You should see a full-width "note" style message under the page heading, "Warning: Any changes..." - Click the "Reset mappings" button at the bottom of the page. The confirmation alert you see should appear above the note-style message. - I'm not sure how to trigger an "error" type message on this page, but you can test the updated style by going to Administration -> Currencies and submit a new currency with the same name and symbol as one which already exists. Created attachment 117282 [details] [review] Bug 27695: Fix style of messages on Elasticsearch configuration page This patch makes minor modifications to SCSS and HTML to make messages on the Elasticsearch configuration page more consistent with other interfaces. To test you should have Elasticsearch selected as your search engine. - Apply the patch and rebuild the staff interface CSS (https://wiki.koha-community.org/wiki/Working_with_SCSS_in_the_OPAC_and_staff_client) - Go to Administration -> Search engine configuration (Elasticsearch) - You should see a full-width "note" style message under the page heading, "Warning: Any changes..." - Click the "Reset mappings" button at the bottom of the page. The confirmation alert you see should appear above the note-style message. - I'm not sure how to trigger an "error" type message on this page, but you can test the updated style by going to Administration -> Currencies and submit a new currency with the same name and symbol as one which already exists. Signed-off-by: Michal Denar <black23@gmail.com> Hmm.. are 'alert' and an 'error' really the same...? I feel like we should have a clearer global idea of what 'message', 'info', 'alert', 'error' etc mean.. and I'm not even sure that's the definitive list :| (In reply to Martin Renvoize from comment #3) > Hmm.. are 'alert' and an 'error' really the same...? I don't know that they are from the point of view of the user. At some point many scripts where modified to use a different method for sending messages to the template. My searches are failing me so I can't find the relevant bug number. When this change was made, many scripts were updated so that the template would display a dialog with an "error" class which was not defined at the time: push @messages, {type => 'error', code => 'error_on_delete' }; From the way this change was made I suspect the use of "error" was intended to serve the same purpose as "alert." In the staff client there are only "message" and "alert" type dialogs in use. In some places the script has been changed to use the previously-standard "alert," e.g. Bug 22575. For what it's worth... The "message" style was originally defined for any kind of message that is informational, i.e. no action is required on the part of the user. <div class="dialog message"> ... </div> The "alert" style was intended for messages which DO require the user to take action: They have to make a choice asked in the dialog, or they have to address the error in the dialog in order to proceed. <div class="dialog alert"> ... </div> The "error" parameter being added to scripts resulted in this markup: <div class="dialog error"> ... </div> ...which was never added as a style in CSS. Thanks for clarifying Owen.. so this patch will actually fix a whole series of 'error' type dialogue boxes.. cool. I was imagining we could go with something along the lines of Bootstraps native set of colour options.. primary, success, info, warning, danger But these don't entirely lend themselves to our use case here. Created attachment 117569 [details] [review] Bug 27695: Fix style of messages on Elasticsearch configuration page This patch makes minor modifications to SCSS and HTML to make messages on the Elasticsearch configuration page more consistent with other interfaces. To test you should have Elasticsearch selected as your search engine. - Apply the patch and rebuild the staff interface CSS (https://wiki.koha-community.org/wiki/Working_with_SCSS_in_the_OPAC_and_staff_client) - Go to Administration -> Search engine configuration (Elasticsearch) - You should see a full-width "note" style message under the page heading, "Warning: Any changes..." - Click the "Reset mappings" button at the bottom of the page. The confirmation alert you see should appear above the note-style message. - I'm not sure how to trigger an "error" type message on this page, but you can test the updated style by going to Administration -> Currencies and submit a new currency with the same name and symbol as one which already exists. Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> After our discussion, I'm happy to call this PQA.. It fixes a class of missing styling in one hit. I still think we could perhaps make a distinction between 'needs attention from staff user' and 'needs attention from administrator' (error vs alert) later down the line perhaps. Passing QA (In reply to Martin Renvoize from comment #9) > I still think we could perhaps make a > distinction between 'needs attention from staff user' and 'needs attention > from administrator' (error vs alert) later down the line perhaps. I agree. I was going to suggest that the distinction be made between "requires the user to make a choice in order to proceed," and "the operation cannot be completed at all for some reason." (In reply to Owen Leonard from comment #10) > I agree. I was going to suggest that the distinction be made between > "requires the user to make a choice in order to proceed," and "the operation > cannot be completed at all for some reason." I've added it to the dev meeting agenda, thanks Owen :) (In reply to Martin Renvoize from comment #6) > Thanks for clarifying Owen.. so this patch will actually fix a whole series > of 'error' type dialogue boxes.. cool. Well, yes, maybe. or not. $ git grep 'class="error"' returns a long non-exhaustive list Quick random test: https://snipboard.io/px5fq4.jpg When receiving, before: https://snipboard.io/jSWMmf.jpg after: https://snipboard.io/AOoUpa.jpg And certainly many more. I don't think we should do that on this bug report. Oops, my bad, I misread the patch. I think this should only apply to 'message boxes' for the moment.. i.e. .message .alert, .message .error ? Created attachment 119428 [details] [review] Bug 27695: (follow-up) Add specificity to CSS changes This patch moves the definition of .alert and .error classes into the block for .dialog in order to make sure they don't have an effect beyond the desired scope. The ".closebtn" section is redundant and has been removed. Created attachment 119550 [details] [review] Bug 27695: Fix style of messages on Elasticsearch configuration page This patch makes minor modifications to SCSS and HTML to make messages on the Elasticsearch configuration page more consistent with other interfaces. To test you should have Elasticsearch selected as your search engine. - Apply the patch and rebuild the staff interface CSS (https://wiki.koha-community.org/wiki/Working_with_SCSS_in_the_OPAC_and_staff_client) - Go to Administration -> Search engine configuration (Elasticsearch) - You should see a full-width "note" style message under the page heading, "Warning: Any changes..." - Click the "Reset mappings" button at the bottom of the page. The confirmation alert you see should appear above the note-style message. - I'm not sure how to trigger an "error" type message on this page, but you can test the updated style by going to Administration -> Currencies and submit a new currency with the same name and symbol as one which already exists. Signed-off-by: Michal Denar <black23@gmail.com> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com> Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Created attachment 119551 [details] [review] Bug 27695: (follow-up) Add specificity to CSS changes This patch moves the definition of .alert and .error classes into the block for .dialog in order to make sure they don't have an effect beyond the desired scope. The ".closebtn" section is redundant and has been removed. Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Pushed to master for 21.05, thanks to everybody involved! Pushed to 20.11.x for 20.11.06 Pushed to 20.05.x for 20.05.12 Not backported to oldoldstable (19.11.x). Feel free to ask if it's needed. |