-
-
Notifications
You must be signed in to change notification settings - Fork 8.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow linking to pre-filtered plugin manager pages, link from labels/… #4591
Conversation
@@ -77,7 +77,9 @@ THE SOFTWARE. | |||
<div class="plugin-manager__categories"> | |||
<j:forEach var="label" items="${p.categories}"> | |||
<span class="plugin-manager__category-label"> | |||
${app.updateCenter.getCategoryDisplayName(label)} | |||
<a href="?filter=${app.updateCenter.getCategoryDisplayName(label)}"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wow, love that these can be made into hyperlinks. I'd say to inspire us in bootstrap pill buttons (https://getbootstrap.com/docs/4.4/components/badge/#links) and maybe style the background and remove the text underline. I can get some styles going on if you want.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I already expected something like that, but it's a waste of time for me to start on before any feedback 😆 Go for it 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will do, may take a few days if that's OK
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we remove the on-hold label on this one, now that my changes are merged in?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, thanks for the reminder @fqueiruga
Add custom hyperlink styles for the category pills
This seems to cause a page refresh and all plugins are shown for a couple of seconds and then the filter is applied, a couple of possible fixs:
|
Right. Someone else can figure out how to do this via JS. Only ever a problem on the "Available" tab, and only if we reject #4580, as the others will have far fewer entries. |
Ok +1 once #4580 is merged |
I'm assuming this one is ready to go once the conflicts are resolved right? |
Pretty much, I would like to test it out locally though. |
…s into linkable-filter-field
New PR comment addition:
|
This doesn't seem to be working right for me on the available tab, if I click a label it puts the query in the input and focuses it but doesn't do anything till I move the cursor or press the arrow key one to the right. The available tab seems to be working fine |
Where does it work and where doesn't it? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Thanks for the PR, nice improvement Let’s merge this in 24 hours if no negative feedback |
I think I got some feedback on #4534 somewhere that said the tags/labels should be clickable. Now they are.
This has an interaction with #4580: That PR would show more plugins for a given nontrivial expression, as spaces are separators there, rather than part of a single search term.
Additionally, it might be confusing for users who expect a click on a label to only show plugins with the label to also get results that match the label as a search expression, because that's what this does. Thoughts?
Screenshot
See URL and filter field:
(Note that this is my update center test data from #4588, so only has plugins whose name starts with "a" or "b".)
Proposed changelog entries
Proposed upgrade guidelines
N/A
Submitter checklist
Proposed changelog entries
section only if there are breaking changes or other changes which may require extra steps from users during the upgradeDesired reviewers
@mention
Maintainer checklist
Before the changes are marked as
ready-for-merge
:Proposed changelog entries
are correctupgrade-guide-needed
label is set and there is aProposed upgrade guidelines
section in the PR title. (example)lts-candidate