Bug 35134 - cn_browser.pl pop-up size should be configurable
Summary: cn_browser.pl pop-up size should be configurable
Status: Signed Off
Alias: None
Product: Koha
Classification: Unclassified
Component: Cataloging (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement
Assignee: Pedro Amorim
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2023-10-23 16:02 UTC by Lucas Gass (lukeg)
Modified: 2024-09-24 16:23 UTC (History)
6 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:
Circulation function:


Attachments
Bug 35134: Make cn_browser popup responsive to screen size (1.57 KB, patch)
2023-11-21 15:28 UTC, Lucas Gass (lukeg)
Details | Diff | Splinter Review
Bug 35134: Make cn_browser popup responsive to screen size (1.62 KB, patch)
2023-11-21 17:25 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 35134: [Alternative] Save popup size to cookies and use that instead (1.85 KB, patch)
2024-04-23 12:21 UTC, Pedro Amorim
Details | Diff | Splinter Review
Bug 35134: Make cn_browser popup responsive to screen size (1.67 KB, patch)
2024-05-30 05:15 UTC, David Nind
Details | Diff | Splinter Review
Bug 35134: [Alternative] Save popup size to cookies and use that instead (1.89 KB, patch)
2024-05-30 05:15 UTC, David Nind
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Lucas Gass (lukeg) 2023-10-23 16:02:11 UTC
To recreate:

1. Use the cn_browser.pl plugin for the 952$o ( call number )
2. Add or edit an item and select the `...` to the right of the call number field.
3. The pop-up is hardcoded as 500px x 400px pop-up. 
4. Some librarians would like to be able to change the size of this.
Comment 1 Sarah Cornell 2023-10-23 17:27:29 UTC
*raises hand*  That's us.  The call number browser popup needs to require fewer, if any, user interactions to show the searched-for call number in the range.   

It currently returns 30 results with the search term in the center, at row 16.  The results are always longer than the current popup size.  As a result, every lookup requires scrolling down to see the results.

Rather than pick a larger size that hopes to serve most monitor sizes, it would be better to make the popup size configurable or responsive to the monitor size.
Comment 2 Lucas Gass (lukeg) 2023-11-21 15:28:28 UTC
Created attachment 159169 [details] [review]
Bug 35134: Make cn_browser popup responsive to screen size

To test:
1. Use the cn_browser.pl plugin for the 952$o ( call number )
2. Add or edit an item and select the `...` to the right of the call number field.
3. The pop-up is hardcoded as 500px x 400px pop-up.
4. APPLY PATCH
5. Clear browser cache and try again. The window width should now be half of the current screen size. The window height should be 100% of the screen height.
Comment 3 Owen Leonard 2023-11-21 17:25:44 UTC
Created attachment 159171 [details] [review]
Bug 35134: Make cn_browser popup responsive to screen size

To test:
1. Use the cn_browser.pl plugin for the 952$o ( call number )
2. Add or edit an item and select the `...` to the right of the call
   number field.
3. The pop-up is hardcoded as 500px x 400px pop-up.
4. APPLY PATCH
5. Clear browser cache and try again. The window width should now be
   half of the current screen size. The window height should be 100% of
   the screen height.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 4 Phil Ringnalda 2024-02-02 21:51:38 UTC
Sizing relative to screen width/height gives reasonable results on a laptop screen (where I'm typically using window size close to screen size), but on a decent size monitor it gets pretty comical. I don't have a monitor quite big enough to catalog in a window that's less than half the screen width, but plenty of people do, which would result in a popup that completely covers the parent window. And since the plugin only displays a fixed number of results, screen height makes it look like a mistake, with two-thirds of the window height just blank.

Half the parent window width and the parent window height would eliminate covering the parent window, and give people with more space who need it bigger an easy way to get it: just increase the size of the window you are cataloging in and the popups will be bigger.

Makes me wonder what screen height is for a Safari window in VisionOS.
Comment 5 Pedro Amorim 2024-04-23 12:21:20 UTC
Created attachment 165381 [details] [review]
Bug 35134: [Alternative] Save popup size to cookies and use that instead

Follow the original test-plan, open the pop-up and resize to whatever size.
Close the pop-up, open again, notice it opens to the same size it was previously closed.
Comment 6 Pedro Amorim 2024-04-23 12:21:32 UTC
Hey guys, looking here. My goal is to contribute (not block) so if the following doesn't make sense or you disagree, please ignore.

I'm not a fan of the proposed solution because it's changing arbitrary values for different arbitrary values (although relative to the screen) -> it does not look like a definitive fix. Having said that, I understand it and I believe the reason behind is that window popups are a bit of a wild west, in my opinion (see bug 36143).

As an alternative proof of concept, I'm proposing we use a default window size but also save whatever resize the user applies to the pop-up and use that going forward. This way, ideally, any user can resize to whatever size feels comfortable to them and when they open the popup again it'll open the window using those size values instead.

My suggestion is not meant to be the end all be all for window pop ups, but rather to stimulate discussion and brainstorming around this.
Comment 7 Lucas Gass (lukeg) 2024-04-23 13:49:15 UTC
(In reply to Pedro Amorim from comment #6)
> Hey guys, looking here. My goal is to contribute (not block) so if the
> following doesn't make sense or you disagree, please ignore.
> 
> I'm not a fan of the proposed solution because it's changing arbitrary
> values for different arbitrary values (although relative to the screen) ->
> it does not look like a definitive fix. Having said that, I understand it
> and I believe the reason behind is that window popups are a bit of a wild
> west, in my opinion (see bug 36143).
> 
> As an alternative proof of concept, I'm proposing we use a default window
> size but also save whatever resize the user applies to the pop-up and use
> that going forward. This way, ideally, any user can resize to whatever size
> feels comfortable to them and when they open the popup again it'll open the
> window using those size values instead.
> 
> My suggestion is not meant to be the end all be all for window pop ups, but
> rather to stimulate discussion and brainstorming around this.

I'm for this idea. I didn't test because your alternative patch doesn't seem to apply.
Comment 8 Pedro Amorim 2024-04-23 13:55:41 UTC
(In reply to Lucas Gass from comment #7)
> (In reply to Pedro Amorim from comment #6)
> > Hey guys, looking here. My goal is to contribute (not block) so if the
> > following doesn't make sense or you disagree, please ignore.
> > 
> > I'm not a fan of the proposed solution because it's changing arbitrary
> > values for different arbitrary values (although relative to the screen) ->
> > it does not look like a definitive fix. Having said that, I understand it
> > and I believe the reason behind is that window popups are a bit of a wild
> > west, in my opinion (see bug 36143).
> > 
> > As an alternative proof of concept, I'm proposing we use a default window
> > size but also save whatever resize the user applies to the pop-up and use
> > that going forward. This way, ideally, any user can resize to whatever size
> > feels comfortable to them and when they open the popup again it'll open the
> > window using those size values instead.
> > 
> > My suggestion is not meant to be the end all be all for window pop ups, but
> > rather to stimulate discussion and brainstorming around this.
> 
> I'm for this idea. I didn't test because your alternative patch doesn't seem
> to apply.

Sorry, was built on top of yours. It should perhaps have been it's own standalone thing but wanted to keep your patch as the default initial size, and extend it. Try applying both, it should work.
Comment 9 Lucas Gass (lukeg) 2024-05-29 17:26:11 UTC
> Sorry, was built on top of yours. It should perhaps have been it's own
> standalone thing but wanted to keep your patch as the default initial size,
> and extend it. Try applying both, it should work.

I've applied both and I like how this works, I am resetting this to NSO. Thanks Pedro!
Comment 10 David Nind 2024-05-30 05:15:10 UTC
Created attachment 167261 [details] [review]
Bug 35134: Make cn_browser popup responsive to screen size

To test:
1. Use the cn_browser.pl plugin for the 952$o ( call number )
2. Add or edit an item and select the `...` to the right of the call
   number field.
3. The pop-up is hardcoded as 500px x 400px pop-up.
4. APPLY PATCH
5. Clear browser cache and try again. The window width should now be
   half of the current screen size. The window height should be 100% of
   the screen height.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: David Nind <david@davidnind.com>
Comment 11 David Nind 2024-05-30 05:15:12 UTC
Created attachment 167262 [details] [review]
Bug 35134: [Alternative] Save popup size to cookies and use that instead

Follow the original test-plan, open the pop-up and resize to whatever size.
Close the pop-up, open again, notice it opens to the same size it was previously closed.

Signed-off-by: David Nind <david@davidnind.com>
Comment 12 David Nind 2024-05-30 05:16:37 UTC
Testing notes (using KTD):

1. Change the framework to add the cn_browser.pl as a plugin for 952$o (I edit the BKS framework)
Comment 13 Kyle M Hall (khall) 2024-07-26 17:28:36 UTC
I love Pedro's patch. I think maybe we should use browser storage instead of cookies though! That way it won't get cleared if  the cookies are cleared.
Comment 14 Lucas Gass (lukeg) 2024-09-24 16:23:33 UTC
I like how this works and don't have a problem with it being cookies vs local storage. 

I wrote the initial patch so I'll excuse myself from QAing. :)