Bug 18551 - Hide with CSS dynamic elements in member search
Summary: Hide with CSS dynamic elements in member search
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Patrons (show other bugs)
Version: master
Hardware: All All
: P5 - low minor (vote)
Assignee: Fridolin Somers
QA Contact: Testopia
URL:
Keywords:
: 18518 (view as bug list)
Depends on:
Blocks: 18685 19305
  Show dependency treegraph
 
Reported: 2017-05-05 14:24 UTC by Fridolin Somers
Modified: 2018-12-03 20:03 UTC (History)
9 users (show)

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


Attachments
Bug 18551 - Hide with CSS dynamic elements in member search (2.85 KB, patch)
2017-05-05 14:31 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 18551: followup - hide advanced filters in header, move hidding to css file (3.08 KB, patch)
2017-05-09 13:28 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 18551 - Hide with CSS dynamic elements in member search (3.04 KB, patch)
2017-05-10 04:12 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 18551: followup - hide advanced filters in header, move hidding to css file (3.25 KB, patch)
2017-05-10 04:14 UTC, Marc Véron
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 18551 - Hide with CSS dynamic elements in member search (3.11 KB, patch)
2017-05-16 19:55 UTC, Philippe Audet-Fortin
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 18551: followup - hide advanced filters in header, move hidding to css file (3.33 KB, patch)
2017-05-16 19:55 UTC, Philippe Audet-Fortin
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 18551 - Hide with CSS dynamic elements in member search (3.11 KB, patch)
2017-05-16 19:56 UTC, Philippe Audet-Fortin
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 18551: followup - hide advanced filters in header, move hidding to css file (3.33 KB, patch)
2017-05-16 19:57 UTC, Philippe Audet-Fortin
Details | Diff | Splinter Review
Bug 18551 - Hide with CSS dynamic elements in member search (3.17 KB, patch)
2017-05-18 19:51 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18551: followup - hide advanced filters in header, move hidding to css file (3.40 KB, patch)
2017-05-18 19:51 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 18551: [16.11.x] followup - hide advanced filters in header, move hidding to css file (3.44 KB, patch)
2017-08-03 08:21 UTC, Fridolin Somers
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Fridolin Somers 2017-05-05 14:24:00 UTC
In member search page, the result table is in Ajax so fully managed by Javascript. There is also a yellow dialog message prepared in HTML.

Thoses elements are hidden by JS code : ie $("#patron_list_dialog").hide().

The problem is that the static page is first loaded an displayed then the JS code runs an hides the elements.
On a low performance computer, this action is visible and looks like there is a blinking yellow message.

I propose to hide with CSS so that thoses elements are not displayed in static page and are there shown in dynamic JS code.
Comment 1 Fridolin Somers 2017-05-05 14:31:18 UTC
Created attachment 63171 [details] [review]
Bug 18551 - Hide with CSS dynamic elements in member search

In member search page, the result table is in Ajax so fully managed by Javascript. There is also a yellow dialog message prepared in HTML.

Thoses elements are hidden by JS code : ie $("#patron_list_dialog").hide().

The problem is that the static page is first loaded an displayed then the JS code runs an hides the elements.
On a low performance computer, this action is visible and looks like there is a blinking yellow message.

I propose to hide with CSS so that thoses elements are not displayed in static page and are there shown in dynamic JS code.

Test plan :
Check display is unchanged :
- Go to home page /cgi-bin/koha/members/members-home.pl
- Perform patron search from header search box
- Perform patron search by clicking on a letter
- Perform patron search from filters (left of results table)
- Select a patron and add it to a list => you see the yellow message
Comment 2 Fridolin Somers 2017-05-05 15:59:53 UTC
Ah maybe depends on 18518
Comment 3 Marc Véron 2017-05-06 09:00:58 UTC
*** Bug 18518 has been marked as a duplicate of this bug. ***
Comment 4 Marc Véron 2017-05-06 09:40:51 UTC
I tested but did not yet sign off because this bug is marked as 'In discussion'

With this patch, two of the reasons are gone (yellow dialogue and data table).

I think there is one more thing to be addressed: The form for the search filters shortly displays and then hides again. It moves the whole page down and up again. Do you have an idea how to fix it?
Comment 5 Liz Rea 2017-05-09 01:21:29 UTC
This is happening all over, and I think possibly it's more to do with when the JS is loading.

Maybe instead of document.ready, we need document.onload?
Comment 6 Fridolin Somers 2017-05-09 07:08:12 UTC
(In reply to Liz Rea from comment #5)
> This is happening all over, and I think possibly it's more to do with when
> the JS is loading.
> 
> Maybe instead of document.ready, we need document.onload?

Ah, I dont know that.
I think we should has this on koha-dev.

But I think this patch will always be usefull because it converts existing hide() JS call into CSS, which is more performant.
Comment 7 Josef Moravec 2017-05-09 13:28:54 UTC
Created attachment 63287 [details] [review]
Bug 18551: followup - hide advanced filters in header, move hidding to css file

Test plan:
The same as first patch, but also with advanced search form in header hidden
on page load - see comment 4
Comment 8 Marc Véron 2017-05-10 04:12:21 UTC
Created attachment 63314 [details] [review]
Bug 18551 - Hide with CSS dynamic elements in member search

In member search page, the result table is in Ajax so fully managed by Javascript. There is also a yellow dialog message prepared in HTML.

Thoses elements are hidden by JS code : ie $("#patron_list_dialog").hide().

The problem is that the static page is first loaded an displayed then the JS code runs an hides the elements.
On a low performance computer, this action is visible and looks like there is a blinking yellow message.

I propose to hide with CSS so that thoses elements are not displayed in static page and are there shown in dynamic JS code.

Test plan :
Check display is unchanged :
- Go to home page /cgi-bin/koha/members/members-home.pl
- Perform patron search from header search box
- Perform patron search by clicking on a letter
- Perform patron search from filters (left of results table)
- Select a patron and add it to a list => you see the yellow message

Yellow message does no longer appear with this patch.
Signed-off-by: Marc Véron <veron@veron.ch>
Comment 9 Marc Véron 2017-05-10 04:14:39 UTC
Created attachment 63315 [details] [review]
Bug 18551: followup - hide advanced filters in header, move hidding to css file

Test plan:
The same as first patch, but also with advanced search form in header hidden
on page load - see comment 4

Issue with advanced search form is gone.
Signed-off-by: Marc Véron <veron@veron.ch>
Comment 10 Marc Véron 2017-05-10 04:31:10 UTC
Two issues are addressed with first two patches, I signed the patches off.

But there is one more problem showing up (It was hidden by the first two problems):

To reproduce:

- Apply first two patches

- Go to Home > Patrons

- Have a close look to the tabs Search Patrons / Check out / Check in ...
- You will see that first there is no green line around the tabs and the
  first tab 'Search patrons' is not in bold. The height of the whole search
  part changes, moving the rest of the page contents up. Then the green line 
  appears, font of 'Search patron' changes to bold, the area of the search part
  expands and the rest of the page moves down again.

- You can reproduce by performing a search as well
Comment 11 Philippe Audet-Fortin 2017-05-15 15:08:59 UTC
Hi Marc,

Can you specify the browser you are using and its version, because I can't reproduce the problem.
Comment 12 Marc Véron 2017-05-15 15:28:44 UTC
(In reply to Philippe Audet-Fortin from comment #11)
> Hi Marc,
> 
> Can you specify the browser you are using and its version, because I can't
> reproduce the problem.

Firefox 53.0 (Windows)
Firefox ESR 45.3.0 (Debian)

See following comment as well (slow down with toogle(500):
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18518#c5
Comment 13 Fridolin Somers 2017-05-16 13:17:26 UTC
Looks nice to me.
It happens very fast on a good computer so it is not easy to reproduce.
Use an old Pentium II ;)
Comment 14 Philippe Audet-Fortin 2017-05-16 19:55:04 UTC
Created attachment 63489 [details] [review]
[SIGNED-OFF] Bug 18551 - Hide with CSS dynamic elements in member search

In member search page, the result table is in Ajax so fully managed by Javascript. There is also a yellow dialog message prepared in HTML.

Thoses elements are hidden by JS code : ie $("#patron_list_dialog").hide().

The problem is that the static page is first loaded an displayed then the JS code runs an hides the elements.
On a low performance computer, this action is visible and looks like there is a blinking yellow message.

I propose to hide with CSS so that thoses elements are not displayed in static page and are there shown in dynamic JS code.

Test plan :
Check display is unchanged :
- Go to home page /cgi-bin/koha/members/members-home.pl
- Perform patron search from header search box
- Perform patron search by clicking on a letter
- Perform patron search from filters (left of results table)
- Select a patron and add it to a list => you see the yellow message

Yellow message does no longer appear with this patch.
Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Philippe <philippe.audet-fortin@inlibro.com>
Comment 15 Philippe Audet-Fortin 2017-05-16 19:55:12 UTC
Created attachment 63490 [details] [review]
[SIGNED-OFF] Bug 18551: followup - hide advanced filters in header, move hidding to css file

Test plan:
The same as first patch, but also with advanced search form in header hidden
on page load - see comment 4

Issue with advanced search form is gone.
Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Philippe <philippe.audet-fortin@inlibro.com>
Comment 16 Philippe Audet-Fortin 2017-05-16 19:56:46 UTC
Created attachment 63491 [details] [review]
[SIGNED-OFF] Bug 18551 - Hide with CSS dynamic elements in member search

In member search page, the result table is in Ajax so fully managed by Javascript. There is also a yellow dialog message prepared in HTML.

Thoses elements are hidden by JS code : ie $("#patron_list_dialog").hide().

The problem is that the static page is first loaded an displayed then the JS code runs an hides the elements.
On a low performance computer, this action is visible and looks like there is a blinking yellow message.

I propose to hide with CSS so that thoses elements are not displayed in static page and are there shown in dynamic JS code.

Test plan :
Check display is unchanged :
- Go to home page /cgi-bin/koha/members/members-home.pl
- Perform patron search from header search box
- Perform patron search by clicking on a letter
- Perform patron search from filters (left of results table)
- Select a patron and add it to a list => you see the yellow message

Yellow message does no longer appear with this patch.
Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Philippe <philippe.audet-fortin@inlibro.com>
Comment 17 Philippe Audet-Fortin 2017-05-16 19:57:00 UTC
Created attachment 63492 [details] [review]
[SIGNED-OFF] Bug 18551: followup - hide advanced filters in header, move hidding to css file

Test plan:
The same as first patch, but also with advanced search form in header hidden
on page load - see comment 4

Issue with advanced search form is gone.
Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Philippe <philippe.audet-fortin@inlibro.com>
Comment 18 Jonathan Druart 2017-05-18 19:51:18 UTC
Created attachment 63546 [details] [review]
Bug 18551 - Hide with CSS dynamic elements in member search

In member search page, the result table is in Ajax so fully managed by Javascript. There is also a yellow dialog message prepared in HTML.

Thoses elements are hidden by JS code : ie $("#patron_list_dialog").hide().

The problem is that the static page is first loaded an displayed then the JS code runs an hides the elements.
On a low performance computer, this action is visible and looks like there is a blinking yellow message.

I propose to hide with CSS so that thoses elements are not displayed in static page and are there shown in dynamic JS code.

Test plan :
Check display is unchanged :
- Go to home page /cgi-bin/koha/members/members-home.pl
- Perform patron search from header search box
- Perform patron search by clicking on a letter
- Perform patron search from filters (left of results table)
- Select a patron and add it to a list => you see the yellow message

Yellow message does no longer appear with this patch.
Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Philippe <philippe.audet-fortin@inlibro.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 19 Jonathan Druart 2017-05-18 19:51:23 UTC
Created attachment 63547 [details] [review]
Bug 18551: followup - hide advanced filters in header, move hidding to css file

Test plan:
The same as first patch, but also with advanced search form in header hidden
on page load - see comment 4

Issue with advanced search form is gone.
Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Philippe <philippe.audet-fortin@inlibro.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 20 Kyle M Hall 2017-05-19 14:50:49 UTC
Pushed to master for 17.05, thanks Frido, Josef!
Comment 21 Katrin Fischer 2017-05-21 21:46:05 UTC
These patches have been pushed to 16.11.x and will be in 16.11.08.
Comment 22 Mason James 2017-05-24 03:16:51 UTC
Pushed to 16.05.x, for 16.05.13 release
Comment 23 Katrin Fischer 2017-06-05 13:56:12 UTC
After talking to Owen about a possible solution, I am reverting these patches from 16.11.x, as they cause a display problem with the floating toolbar for patrons (see bug 18685).

Mason, you might want to check if you want to revert as well!
Comment 24 Katrin Fischer 2017-06-21 06:21:27 UTC
As we have a bug fix now for the problem with the floating toolbar (bug 18685) I'd give this another go for 16.11.x, but patches don't apply cleanly. Please check!
Comment 25 Katrin Fischer 2017-07-16 12:54:59 UTC
Please rebase for 16.11.x if you want to see this and the bugs its blocking included in 16.11.x!
Comment 26 Fridolin Somers 2017-08-03 08:21:58 UTC
Created attachment 65452 [details] [review]
Bug 18551: [16.11.x] followup - hide advanced filters in header, move hidding to css file

Test plan:
The same as first patch, but also with advanced search form in header hidden
on page load - see comment 4

Issue with advanced search form is gone.
Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Philippe <philippe.audet-fortin@inlibro.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 27 Fridolin Somers 2017-08-03 08:23:58 UTC
(In reply to Katrin Fischer from comment #25)
> Please rebase for 16.11.x if you want to see this and the bugs its blocking
> included in 16.11.x!

Patch does not apply simply because lines are added at the end of staff-global.css and since last lines are not the same in master the patch is lost ;)
Comment 28 Fridolin Somers 2017-08-03 08:28:09 UTC
(In reply to Fridolin SOMERS from comment #27)
> (In reply to Katrin Fischer from comment #25)
> > Please rebase for 16.11.x if you want to see this and the bugs its blocking
> > included in 16.11.x!
> 
> Patch does not apply simply because lines are added at the end of
> staff-global.css and since last lines are not the same in master the patch
> is lost ;)

Note that you must clear cache after applying the patch (Ctrl+F5).
Tested on Firefox 54.
Comment 29 Katrin Fischer 2017-08-16 21:17:48 UTC
Thx Fridolin for providing the patch! 

This patch has been pushed to 16.11.x and will be in 16.11.11.
Comment 30 Mason James 2017-08-24 14:07:30 UTC
Pushed to 16.05.x, for 16.05.16 release
Comment 31 Mason James 2017-09-05 10:34:59 UTC
(In reply to Katrin Fischer from comment #29)
> Thx Fridolin for providing the patch! 
> 
> This patch has been pushed to 16.11.x and will be in 16.11.11.

hi Cait

i think 16.11.11 is missing this patch..
 https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=63546

..currently patron search seems broken
Comment 32 Katrin Fischer 2017-09-10 09:11:09 UTC
Mason, you were right. Thx a lot for catching this!

Missing patch has been pushed to 16.11.x and will be in 16.11.12.
Comment 33 Mason James 2017-09-20 02:02:15 UTC
(In reply to Katrin Fischer from comment #32)
> Mason, you were right. Thx a lot for catching this!
> 
> Missing patch has been pushed to 16.11.x and will be in 16.11.12.

Pushed to 16.05.17 release too