With bug 11088 we have now 3 different places to put submit buttons: 1. syspref and cataloguing: floating toolbar, top left 2. patron add form (and soon item edit form): floating button, bottom right 3. all others: bottom left I think the floating elements are a good idea, to avoid unnecessary scrolling, but I don't think we need as many different places to put them.
And in some places, fixed at the top. For example, when editing MARC authority record subfields: /cgi-bin/koha/admin/auth_subfields_structure.pl?op=add_form&tagfield=000&authtypecode=
Created attachment 51043 [details] [review] Bug 16400: Update the fixFloat plugin I have updated it just in case it fixed the issue. I don't think there are big changes between our current version and this one, so keep it up-to-date
Created attachment 51044 [details] [review] Bug 16400: Let's hack the fixFloat plugin to fix our needs On comment bug 11088 comment 2, Owen explained the problem with the fixFloat toolbar. Since we have a hidden div, the plugin does not manage to place the toolbar correctly. In order to make it work, this patch adds some changes to the plugin to add the ability to define our own offset. When the div is extended (+, filteraction_on), the offset should be updated to move the toolbar. Same when the div is folded (-, filteraction_off)
These patches are here for discussion, but it would be good to reach a consensus for the next release.
I'm having some problems with this patch: - It needs to be rebased (although I think I was able to correctly fix the merge problems for testing). - In my tests the toolbar doesn't "stick' to the top of the viewport because it has a margin on it. - When it is floating, the toolbar isn't styled like it is on other pages without this patch. There is a "floating" class which isn't getting added. - It breaks other instances of the floating toolbar. I'm getting a JS error on the cataloging page, for instance. I'm not sure why, but having only "save" and "cancel" in the toolbar doesn't feel right to me. But I agree that we need to be consistent, and I agree that this page really needs some form of always-visible save button.
Created attachment 51144 [details] [review] Bug 16400: Let's hack the fixFloat plugin to fix our needs On comment bug 11088 comment 2, Owen explained the problem with the fixFloat toolbar. Since we have a hidden div, the plugin does not manage to place the toolbar correctly. In order to make it work, this patch adds some changes to the plugin to add the ability to define our own offset. When the div is extended (+, filteraction_on), the offset should be updated to move the toolbar. Same when the div is folded (-, filteraction_off)
Created attachment 51145 [details] [review] Bug 16400: Reintroduce the floating class
(In reply to Owen Leonard from comment #5) > I'm having some problems with this patch: > > - It needs to be rebased (although I think I was able to correctly fix the > merge problems for testing). Done. > - In my tests the toolbar doesn't "stick' to the top of the viewport because > it has a margin on it. > - When it is floating, the toolbar isn't styled like it is on other pages > without this patch. There is a "floating" class which isn't getting added. Last patch reintroduces the floating class. > - It breaks other instances of the floating toolbar. I'm getting a JS error > on the cataloging page, for instance. Fixed, I have updated the second patch. > I'm not sure why, but having only "save" and "cancel" in the toolbar doesn't > feel right to me. But I agree that we need to be consistent, and I agree > that this page really needs some form of always-visible save button. As I said, the patches are here for discussion. I have understood from the discussion on bug 11088 that it's what you needed but you did not manage to implement it.
Adding Aleisha, Tomas and Katrin to the discussion. What are your thoughts about the 2 displays/behaviors?
Created attachment 51449 [details] Toolbar covers drop down menu I'm getting this problem where the syspref toolbar covers the drop down menu. Agreed that it would be nice to have some consistency across the buttons. Having just 'Save' and 'Cancel' in the toolbar alone doesn't bother me too much imo!
Created attachment 51471 [details] [review] Bug 16400: Restore the previous stack order behavior When updating the plugin, the stack order behavior has changed. This patch restores the prevous behavior (not always z-index: 1000)
Aleisha, thanks for testing! The last patch fixes the issue you got.
Hi Jonathan, I like this. The buttons on the member entry page for adding a new patron don't have any space between them, but that's the only thing I could find wrong. Something else did come to mind - I think the cancel buttons should use the fa fa-times icon like many of our other cancel buttons. Thoughts?
Created attachment 51517 [details] [review] Bug 16400: Use the fa-times icon for Cancel
Created attachment 51546 [details] [review] [SIGNED-OFF] Bug 16400: Update the fixFloat plugin I have updated it just in case it fixed the issue. I don't think there are big changes between our current version and this one, so keep it up-to-date Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 51547 [details] [review] [SIGNED-OFF] Bug 16400: Let's hack the fixFloat plugin to fix our needs On comment bug 11088 comment 2, Owen explained the problem with the fixFloat toolbar. Since we have a hidden div, the plugin does not manage to place the toolbar correctly. In order to make it work, this patch adds some changes to the plugin to add the ability to define our own offset. When the div is extended (+, filteraction_on), the offset should be updated to move the toolbar. Same when the div is folded (-, filteraction_off) Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 51548 [details] [review] [SIGNED-OFF] Bug 16400: Reintroduce the floating class Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 51549 [details] [review] [SIGNED-OFF] Bug 16400: Restore the previous stack order behavior When updating the plugin, the stack order behavior has changed. This patch restores the prevous behavior (not always z-index: 1000) Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 51550 [details] [review] [SIGNED-OFF] Bug 16400: Use the fa-times icon for Cancel Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Created attachment 51551 [details] [review] Bug 16400 [Follow-up] Proposal to uniform the placement of submit buttons This follow-up makes some corrections to the markup of the cancel button: - Use <a> instead of <button> to eliminate the need for an "onclick" attribute. - Remove the "cancel" class which adds unwanted padding. - Correct the destination of the cancel button when cancelling during an edit (should redirect to the patron detail page).
This is looking good. So is the purpose of this to have every Save and Submit button a scrolling toolbar? In which cases can we just have the buttons at the bottom? Are we going to add patches with toolbars for all the modules etc. to this bug?
I don't think every save and submit button should be in a scrolling toolbar, but I'm not sure how we should define the rules. Right now the rule is "if the form is big," but I'm not sure how you make that more specific. I think our guidelines should allow for simpler forms to have simpler controls.
Created attachment 51569 [details] [review] Bug 16400 [Follow-up] Proposal to uniform the placement of submit buttons This follow-up makes some corrections to the markup of the cancel button: - Use <a> instead of <button> to eliminate the need for an "onclick" attribute. - Remove the "cancel" class which adds unwanted padding. - Correct the destination of the cancel button when cancelling during an edit (should redirect to the patron detail page). Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 52765 [details] [review] Bug 16400: Update the fixFloat plugin I have updated it just in case it fixed the issue. I don't think there are big changes between our current version and this one, so keep it up-to-date Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 52766 [details] [review] Bug 16400: Let's hack the fixFloat plugin to fix our needs On comment bug 11088 comment 2, Owen explained the problem with the fixFloat toolbar. Since we have a hidden div, the plugin does not manage to place the toolbar correctly. In order to make it work, this patch adds some changes to the plugin to add the ability to define our own offset. When the div is extended (+, filteraction_on), the offset should be updated to move the toolbar. Same when the div is folded (-, filteraction_off) Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 52767 [details] [review] Bug 16400: Reintroduce the floating class Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 52768 [details] [review] Bug 16400: Restore the previous stack order behavior When updating the plugin, the stack order behavior has changed. This patch restores the prevous behavior (not always z-index: 1000) Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 52769 [details] [review] Bug 16400: Use the fa-times icon for Cancel Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Created attachment 52770 [details] [review] Bug 16400 [Follow-up] Proposal to uniform the placement of submit buttons This follow-up makes some corrections to the markup of the cancel button: - Use <a> instead of <button> to eliminate the need for an "onclick" attribute. - Remove the "cancel" class which adds unwanted padding. - Correct the destination of the cancel button when cancelling during an edit (should redirect to the patron detail page). Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org> Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
I like this a lot, the styling feels more consistent with existing functions than the save button on its own.
Pushed to master for 16.11, thanks Jonathan, Owen!
Pushed in 16.05. Will be in 16.05.02.