Bug 16400 - Proposal to uniform the placement of submit buttons
Summary: Proposal to uniform the placement of submit buttons
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Templates (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks: 18305
  Show dependency treegraph
 
Reported: 2016-04-29 18:00 UTC by Jonathan Druart
Modified: 2018-06-04 20:10 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 16400: Update the fixFloat plugin (4.72 KB, patch)
2016-04-29 18:07 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 16400: Let's hack the fixFloat plugin to fix our needs (5.56 KB, patch)
2016-04-29 18:07 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 16400: Let's hack the fixFloat plugin to fix our needs (5.32 KB, patch)
2016-05-03 14:04 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 16400: Reintroduce the floating class (1.11 KB, patch)
2016-05-03 14:05 UTC, Jonathan Druart
Details | Diff | Splinter Review
Toolbar covers drop down menu (36.52 KB, image/png)
2016-05-12 02:36 UTC, Aleisha Amohia
Details
Bug 16400: Restore the previous stack order behavior (1.70 KB, patch)
2016-05-13 14:06 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 16400: Use the fa-times icon for Cancel (1.97 KB, patch)
2016-05-16 10:56 UTC, Jonathan Druart
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 16400: Update the fixFloat plugin (4.83 KB, patch)
2016-05-16 18:19 UTC, Owen Leonard
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 16400: Let's hack the fixFloat plugin to fix our needs (5.42 KB, patch)
2016-05-16 18:19 UTC, Owen Leonard
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 16400: Reintroduce the floating class (1.22 KB, patch)
2016-05-16 18:19 UTC, Owen Leonard
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 16400: Restore the previous stack order behavior (1.81 KB, patch)
2016-05-16 18:19 UTC, Owen Leonard
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 16400: Use the fa-times icon for Cancel (2.08 KB, patch)
2016-05-16 18:19 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 16400 [Follow-up] Proposal to uniform the placement of submit buttons (1.82 KB, patch)
2016-05-16 18:21 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 16400 [Follow-up] Proposal to uniform the placement of submit buttons (1.85 KB, patch)
2016-05-17 13:07 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 16400: Update the fixFloat plugin (4.83 KB, patch)
2016-06-23 21:32 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 16400: Let's hack the fixFloat plugin to fix our needs (5.42 KB, patch)
2016-06-23 21:32 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 16400: Reintroduce the floating class (1.22 KB, patch)
2016-06-23 21:32 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 16400: Restore the previous stack order behavior (1.81 KB, patch)
2016-06-23 21:32 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 16400: Use the fa-times icon for Cancel (2.08 KB, patch)
2016-06-23 21:33 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 16400 [Follow-up] Proposal to uniform the placement of submit buttons (1.90 KB, patch)
2016-06-23 21:34 UTC, Nick Clemens
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2016-04-29 18:00:48 UTC
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.
Comment 1 Owen Leonard 2016-04-29 18:06:53 UTC
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=
Comment 2 Jonathan Druart 2016-04-29 18:07:15 UTC Comment hidden (obsolete)
Comment 3 Jonathan Druart 2016-04-29 18:07:19 UTC Comment hidden (obsolete)
Comment 4 Jonathan Druart 2016-04-29 18:08:08 UTC
These patches are here for discussion, but it would be good to reach a consensus for the next release.
Comment 5 Owen Leonard 2016-05-03 13:16:14 UTC
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.
Comment 6 Jonathan Druart 2016-05-03 14:04:57 UTC Comment hidden (obsolete)
Comment 7 Jonathan Druart 2016-05-03 14:05:00 UTC Comment hidden (obsolete)
Comment 8 Jonathan Druart 2016-05-03 14:07:25 UTC
(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.
Comment 9 Jonathan Druart 2016-05-04 16:29:51 UTC
Adding Aleisha, Tomas and Katrin to the discussion.

What are your thoughts about the 2 displays/behaviors?
Comment 10 Aleisha Amohia 2016-05-12 02:36:58 UTC
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!
Comment 11 Jonathan Druart 2016-05-13 14:06:21 UTC Comment hidden (obsolete)
Comment 12 Jonathan Druart 2016-05-13 14:07:27 UTC
Aleisha, thanks for testing!
The last patch fixes the issue you got.
Comment 13 Aleisha Amohia 2016-05-15 23:51:30 UTC
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?
Comment 14 Jonathan Druart 2016-05-16 10:56:44 UTC Comment hidden (obsolete)
Comment 15 Owen Leonard 2016-05-16 18:19:15 UTC Comment hidden (obsolete)
Comment 16 Owen Leonard 2016-05-16 18:19:18 UTC Comment hidden (obsolete)
Comment 17 Owen Leonard 2016-05-16 18:19:22 UTC Comment hidden (obsolete)
Comment 18 Owen Leonard 2016-05-16 18:19:25 UTC Comment hidden (obsolete)
Comment 19 Owen Leonard 2016-05-16 18:19:29 UTC Comment hidden (obsolete)
Comment 20 Owen Leonard 2016-05-16 18:21:33 UTC Comment hidden (obsolete)
Comment 21 Aleisha Amohia 2016-05-17 00:30:29 UTC
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?
Comment 22 Owen Leonard 2016-05-17 01:51:07 UTC
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.
Comment 23 Jonathan Druart 2016-05-17 13:07:17 UTC Comment hidden (obsolete)
Comment 24 Nick Clemens 2016-06-23 21:32:20 UTC
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>
Comment 25 Nick Clemens 2016-06-23 21:32:26 UTC
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>
Comment 26 Nick Clemens 2016-06-23 21:32:44 UTC
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>
Comment 27 Nick Clemens 2016-06-23 21:32:53 UTC
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>
Comment 28 Nick Clemens 2016-06-23 21:33:03 UTC
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>
Comment 29 Nick Clemens 2016-06-23 21:34:06 UTC
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>
Comment 30 Nick Clemens 2016-06-23 21:36:57 UTC
I like this a lot, the styling feels more consistent with existing functions than the save button on its own.
Comment 31 Kyle M Hall 2016-06-24 13:37:04 UTC
Pushed to master for 16.11, thanks Jonathan, Owen!
Comment 32 Frédéric Demians 2016-08-01 11:54:12 UTC
Pushed in 16.05. Will be in 16.05.02.