Bug 23388 - Make system preferences links
Summary: Make system preferences links
Status: Failed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: Staff Client (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Nick Clemens
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-07-26 18:23 UTC by Nick Clemens
Modified: 2019-10-21 11:46 UTC (History)
6 users (show)

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


Attachments
Bug 23388: Make system preferences links and give them anchors (2.46 KB, patch)
2019-07-26 18:54 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 23388: Sort the tab results when searching (1.58 KB, patch)
2019-07-26 18:54 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 23388: Make system preferences links and give them anchors (2.51 KB, patch)
2019-07-26 21:27 UTC, Maryse Simard
Details | Diff | Splinter Review
Bug 23388: Sort the tab results when searching (1.63 KB, patch)
2019-07-26 21:27 UTC, Maryse Simard
Details | Diff | Splinter Review
Bug 23388: [WIP] Make system preferences links (7.93 KB, patch)
2019-10-17 12:11 UTC, Owen Leonard
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens 2019-07-26 18:23:28 UTC
I would be nice if after searching for a preference and finding it in the list you could easily link to it. Also, if after finding a pref you could see the prefs in the same category
Comment 1 Nick Clemens 2019-07-26 18:54:40 UTC Comment hidden (obsolete)
Comment 2 Nick Clemens 2019-07-26 18:54:42 UTC Comment hidden (obsolete)
Comment 3 Maryse Simard 2019-07-26 21:27:11 UTC
Created attachment 91846 [details] [review]
Bug 23388: Make system preferences links and give them anchors

FIXME: Note the floating toolbar actually breaks this because it hides the anchored pref. Add this CSS to IntrnateUSerCSS to fix it

    padding-top: 50px;
    margin-top: -50px;
}

To test:
1 - Search for 'fields'
2 - Scroll to 'PatronQuickAddFields'
3 - You don't have a way to share a link to this pref
4 - You can't view th other patron prefs without scrolling back to tabs
5 - Apply patch
6 - Repeat
7 - Note the pref is a link you can copy
8 - Click it
9 - You are now on the tab for the pref
10 - Click on other prefs and you focus on them

Signed-off-by: Maryse Simard <maryse.simard@inlibro.com>
Comment 4 Maryse Simard 2019-07-26 21:27:15 UTC
Created attachment 91847 [details] [review]
Bug 23388: Sort the tab results when searching

Before this patch we can get the matching tabs back in any order

To test:
1 - Search system preferences for 'field'
2 - Refresh the page a few times
3 - Note results are reordered
4 - Apply patch
5 - Refresh the page several times again
6 - Results are consistent

Signed-off-by: Maryse Simard <maryse.simard@inlibro.com>
Comment 5 Katrin Fischer 2019-08-08 16:30:45 UTC
Hi Nick,

can you explain this bit please?

FIXME: Note the floating toolbar actually breaks this because it hides the anchored pref. Add this CSS to IntrnateUSerCSS to fix it

    padding-top: 50px;
    margin-top: -50px;
}

Also:

FAIL	koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences.tt
   FAIL	  filters
		missing_filter at line 55 (                <td  id="[% FOREACH NAME IN LINE.NAMES %][% NAME.name %][% END %]" class="name-cell">)
		wrong_html_filter at line 60 (                            <span class="term" id="jumped"><a href="?tab=[% TAB.tab_id %]#[% NAME.name %]">[% NAME.name | html %]</a></span>)
		wrong_html_filter at line 60 (                            <span class="term" id="jumped"><a href="?tab=[% TAB.tab_id %]#[% NAME.name %]">[% NAME.name | html %]</a></span>)
		missing_filter at line 60 (                            <span class="term" id="jumped"><a href="?tab=[% TAB.tab_id %]#[% NAME.name %]">[% NAME.name | html %]</a></span>)
		missing_filter at line 60 (                            <span class="term" id="jumped"><a href="?tab=[% TAB.tab_id %]#[% NAME.name %]">[% NAME.name | html %]</a></span>)
		wrong_html_filter at line 62 (                            <span class="term"><a href="?tab=[% TAB.tab_id %]#[% NAME.name %]">[% NAME.name | html %]</a></span>)
		wrong_html_filter at line 62 (                            <span class="term"><a href="?tab=[% TAB.tab_id %]#[% NAME.name %]">[% NAME.name | html %]</a></span>)
		missing_filter at line 62 (                            <span class="term"><a href="?tab=[% TAB.tab_id %]#[% NAME.name %]">[% NAME.name | html %]</a></span>)
		missing_filter at line 62 (                            <span class="term"><a href="?tab=[% TAB.tab_id %]#[% NAME.name %]">[% NAME.name | html %]</a></span>)
		wrong_html_filter at line 64 (                            <a href="?tab=[% TAB.tab_id %]#[% NAME.name %]">[% NAME.name | html %]</a>)
		wrong_html_filter at line 64 (                            <a href="?tab=[% TAB.tab_id %]#[% NAME.name %]">[% NAME.name | html %]</a>)
		missing_filter at line 64 (                            <a href="?tab=[% TAB.tab_id %]#[% NAME.name %]">[% NAME.name | html %]</a>)
		missing_filter at line 64 (                            <a href="?tab=[% TAB.tab_id %]#[% NAME.name %]">[% NAME.name | html %]</a>)
   FAIL	  forbidden patterns
		forbidden pattern: tab char (line 58)
Comment 6 Owen Leonard 2019-08-08 17:50:51 UTC
I've been working on an alternate patch which addresses one aspect of this bug: Linking easily to specific preferences. 

The proposed patch does two things I don't like: It breaks the previous behavior of the system preference names as <label>. And the links don't make much sense unless you're looking at search results. In other circumstances clicking the link doesn't do anything.

My proposal would be to show a link on hover:

https://zivotdesign.com/p/view.php?p=15646746956636

Unfortunately my solution suffers from the same problem described by Nick: When the page jumps to the anchored link the pref is right at the top of the browser window, thus it's hidden by the floating toolbar.

I worked to come up with a JavaScript solution but failed.
Comment 7 Katrin Fischer 2019-08-10 20:36:29 UTC
How should we continue here? Nick, could you help Owen with his patch or vice versa?
Comment 8 Owen Leonard 2019-10-17 12:11:55 UTC
Created attachment 94368 [details] [review]
Bug 23388: [WIP] Make system preferences links

This alternate solution adds a link which when clicked will copy a URL
linking directly to the corresponding system preference.

To test, apply the patch and view system preferences.

Hovering the mouse over any row in the system preferences table should
trigger the display of an anchor link at the end of the row. Hovering
over the link should display a tooltip, "Copy link to this preference."
Clicking the link should copy it to your clipboard.

Test that the link is copied correctly and that it works to link you to
the preference in question.

Known issues: Because of the way the page is drawn, the browser's
calculation of where to scroll isn't accurate, meaning the target
preference will sometimes be hidden by the floating toolbar.

Question: "Hover" isn't a mobile/touch-friendly interaction. Show the
link all the time? Show it at narrower browser widths?
Comment 9 Ivan Masár 2019-10-17 14:03:36 UTC
Testing the WIP patch in Chrome 77 but I don't see any change. Maybe I'm just holding it wrong...

I see changes in the page source (e.g. <td id="link_). Nothing happens on hover over a preference, though.
Comment 10 Owen Leonard 2019-10-17 14:23:22 UTC
The link should appear all the way at the end of the row, not in the first cell in the row (which, now that I think about it and having seen your patch, is probably the correct place for it).
Comment 11 Ivan Masár 2019-10-17 18:26:51 UTC
Thanks, that was it. I only noticed one flaw - the URL copied to clipboard is relative to the site root, not including scheme, host port. That and after accessing the syspref it is overlaid by the floating toolbar, but there is a fix to that in the previous patch.
Comment 12 Owen Leonard 2019-10-17 18:38:08 UTC
(In reply to Ivan Masár from comment #11)
> Thanks, that was it. I only noticed one flaw - the URL copied to clipboard
> is relative to the site root

Is your staffClientBaseURL system preference set?
Comment 13 Ivan Masár 2019-10-21 07:07:15 UTC
> Is your staffClientBaseURL system preference set?

It's not set. This seems to be the default, both in my installed instance and in kohadevbox, so I think we should not rely on it.
Comment 14 Katrin Fischer 2019-10-21 07:21:52 UTC
(In reply to Ivan Masár from comment #13)
> > Is your staffClientBaseURL system preference set?
> 
> It's not set. This seems to be the default, both in my installed instance
> and in kohadevbox, so I think we should not rely on it.

Koha does rely on it in a lot of places to make things work smoothly. Generally much recommended. For links in staff relativ links might work - but in other places where we link from OPAC to staff and staff to OPAC it's needed.
Comment 15 Owen Leonard 2019-10-21 11:43:02 UTC
(In reply to Katrin Fischer from comment #14)
> Koha does rely on it in a lot of places to make things work smoothly.

(which means we should probably add it to the installer onboarding?)
Comment 16 Ivan Masár 2019-10-21 11:46:55 UTC
Before I found this bug, I made a patch that solves the same problem (linking to sysprefs) in a different way - via search by syspref name rather than anchor links. As a result, my solution shows one syspref, this one scrolls down to the target syspref. See https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23823