-
-
Notifications
You must be signed in to change notification settings - Fork 239
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
Add-on Store: add sub menu items and various improvements #2381
Conversation
c8ef1d9
to
854f67a
Compare
Job #1635: Bundle Size — 11.01MiB (+0.01%).Warning Bundle contains 19 duplicate packages – View duplicate packages Bundle metrics
Bundle size by type
View job #1635 report View jimtng:addons-submenu branch activity View project dashboard |
0d85d26
to
1788712
Compare
What I couldn't figure out: How to push the visits to the sub-sections into the browser's history. Framework7's documentation seems to state that using routable tabs would make that happen automatically, but it doesn't. I tried using |
9b6ed6e
to
d836388
Compare
Signed-off-by: Jimmy Tanagra <jcode@tanagra.id.au>
Signed-off-by: Jimmy Tanagra <jcode@tanagra.id.au>
d836388
to
3b2efdf
Compare
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
@florian-h05 what's the objection for caching the add-ons data? It would get updated anyway so staleness is very minimal. The benefit is a more responsive experience and less twirling circle waiting |
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
I have already written a comment but not submitted review yet, so it is not showing app ... The objection against this way of "caching" is that IMHO caching should be properly implemented on the server side. |
Is there currently a general implementation or mechanism in core for this? |
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.
Overall LGTM, I have commited some changes.
this.currentTab = tab.id | ||
|
||
const section = tab.id === 'main' ? '' : (tab.id + '/') | ||
this.$f7router.updateCurrentUrl('/addons/' + section) |
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.
For my addition, see #2221.
@@ -357,8 +358,7 @@ export default [ | |||
}, | |||
{ | |||
path: 'addons', | |||
beforeEnter: [enforceAdminForRoute], | |||
async: loadAsync(AddonsStorePage), | |||
redirect: '/addons/', |
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.
For what is this required? I have removed it and not noticed any problems.
4778b69
to
2859f6d
Compare
As the REST endpoints have different data sources, there is no general implementation. |
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
2bc3569
to
65e7682
Compare
@jimtng Ready to merge? |
Thanks for the review and fixes. I wish the caching stayed in but hopefully the server side implementation you're working on will work as well. |
Yes thanks |
Hm, for the record, I had mixed feelings about this PR butchering my design for the original add-on store... :/ I guess you'll find the flaws in this design in time. Or not. But I'm okay with having no agency over this as I'm not really active. |
What is your biggest concern with the new design? I personally like it overall and thing especially having a search at each tab is very practical. |
To be honest I wasn't sure of the ability to navigate the add-on store on narrow screens but after all I think it's okay - for now: The bottom toolbar (which I thought initially to be removed) is nice but is becoming quite crowded. |
I added the toolbar for those devices where the sidebar is not permanently available and have also noticed that it is crowded. |
Tbh I've never tried accessing the main UI from a mobile phone, but now I've just tried it. I've noticed several things:
This has been a learning experience for me. I will try to amend this.
|
This is indeed a problem and something that you MUST do before altering designs. |
Resolve #2047
The subsections are implemented as routable tabs, so switching around the different sections is fast.
Installed addons are grouped by category in the main Add-on Store section
Search in the main "Add-on Store" menu will search all types of addons
Search in the corresponding section will only search for add-ons for that category