Skip to content
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

Merged
merged 6 commits into from Feb 23, 2024

Conversation

jimtng
Copy link
Contributor

@jimtng jimtng commented Feb 20, 2024

Resolve #2047

The subsections are implemented as routable tabs, so switching around the different sections is fast.

image

Installed addons are grouped by category in the main Add-on Store section

image

Search in the main "Add-on Store" menu will search all types of addons

image

Search in the corresponding section will only search for add-ons for that category

image image

@jimtng jimtng requested a review from a team as a code owner February 20, 2024 11:47
@jimtng jimtng force-pushed the addons-submenu branch 3 times, most recently from c8ef1d9 to 854f67a Compare February 20, 2024 12:02
Copy link

relativeci bot commented Feb 20, 2024

Job #1635: Bundle Size — 11.01MiB (+0.01%).

65e7682(current) vs bfa2d4a main#1631(baseline)

Warning

Bundle contains 19 duplicate packages – View duplicate packages

Bundle metrics  Change 2 changes Regression 1 regression
                 Current
Job #1635
     Baseline
Job #1631
Regression  Initial JS 1.85MiB(+0.15%) 1.84MiB
No change  Initial CSS 607.89KiB 607.89KiB
Change  Cache Invalidation 23.27% 16.99%
No change  Chunks 220 220
No change  Assets 242 242
No change  Modules 3087 3087
No change  Duplicate Modules 164 164
No change  Duplicate Code 1.77% 1.77%
No change  Packages 150 150
No change  Duplicate Packages 18 18
Bundle size by type  Change 2 changes Regression 2 regressions
                 Current
Job #1635
     Baseline
Job #1631
Regression  JS 9.2MiB (+0.01%) 9.2MiB
Regression  CSS 889.44KiB (~+0.01%) 889.39KiB
Not changed  Fonts 526.1KiB 526.1KiB
Not changed  Media 295.6KiB 295.6KiB
Not changed  IMG 140.74KiB 140.74KiB
Not changed  HTML 1.24KiB 1.24KiB
Not changed  Other 871B 871B

View job #1635 reportView jimtng:addons-submenu branch activityView project dashboard

@jimtng jimtng changed the title Addon-store: add sub menu items and various improvements Add-on Store: add sub menu items and various improvements Feb 20, 2024
@jimtng jimtng force-pushed the addons-submenu branch 2 times, most recently from 0d85d26 to 1788712 Compare February 20, 2024 13:39
@jimtng
Copy link
Contributor Author

jimtng commented Feb 20, 2024

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 history.pushState() but the resulting behaviour is weird, so I took it out.

@jimtng jimtng force-pushed the addons-submenu branch 9 times, most recently from 9b6ed6e to d836388 Compare February 22, 2024 10:21
Signed-off-by: Jimmy Tanagra <jcode@tanagra.id.au>
Signed-off-by: Jimmy Tanagra <jcode@tanagra.id.au>
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
@jimtng
Copy link
Contributor Author

jimtng commented Feb 22, 2024

@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>
@florian-h05
Copy link
Contributor

florian-h05 commented Feb 22, 2024

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.

@jimtng
Copy link
Contributor Author

jimtng commented Feb 22, 2024

caching should be properly implemented on the server side.

Is there currently a general implementation or mechanism in core for this?

Copy link
Contributor

@florian-h05 florian-h05 left a 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)
Copy link
Contributor

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/',
Copy link
Contributor

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.

@florian-h05
Copy link
Contributor

Is there currently a general implementation or mechanism in core for this?

As the REST endpoints have different data sources, there is no general implementation.
Usually, RegistryChangeListeners are used to listen for data changes, but in some cases other ways of "watching" for changes need to be used.
I am currently working on a caching implementation for the add-ons endpoint.

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
@florian-h05
Copy link
Contributor

@jimtng Ready to merge?

@jimtng
Copy link
Contributor Author

jimtng commented Feb 22, 2024

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.

@jimtng
Copy link
Contributor Author

jimtng commented Feb 22, 2024

@jimtng Ready to merge?

Yes thanks

@florian-h05 florian-h05 merged commit 6b7e1c4 into openhab:main Feb 23, 2024
6 checks passed
@florian-h05 florian-h05 added enhancement New feature or request main ui Main UI labels Feb 23, 2024
@florian-h05 florian-h05 added this to the 4.2 milestone Feb 23, 2024
@jimtng jimtng deleted the addons-submenu branch February 23, 2024 06:16
@ghys
Copy link
Member

ghys commented Feb 23, 2024

Hm, for the record, I had mixed feelings about this PR butchering my design for the original add-on store... :/
I just wish I had more time to express them and have a discussion.
(and yes I'm aware of the discussion at https://community.openhab.org/t/ideas-for-new-openhab-installation-experience/154042/).

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.

@florian-h05
Copy link
Contributor

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.
But I’m still unsure what is more practical for navigation: The old tab bar or the side menu entries.

@ghys
Copy link
Member

ghys commented Feb 23, 2024

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:

image

The bottom toolbar (which I thought initially to be removed) is nice but is becoming quite crowded.
This is why I made the add-on store initially about 3 main concepts: bindings, automation, UI (and the rest in a 4th tab, and universal search in a 5th tab).

@florian-h05
Copy link
Contributor

I added the toolbar for those devices where the sidebar is not permanently available and have also noticed that it is crowded.
Having one icon less in the tab bar would already help with the space.

@jimtng
Copy link
Contributor Author

jimtng commented Feb 24, 2024

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:

  • Because the focus goes to the search bar every time I switched to a new tab, it's annoying that the keyboard would pop up an converting up half the screen

  • The suggested and recommended items seem too big and take up a lot of space (same in the old design)

  • I'm not 100% happy with the main "add-ons" tab showing only the installed add-ons. I'd like each section to have a little link or button for "more" that goes to the corresponding tab. But that may be redundant with the bottom tab bar?

  • Now I better understand why having Smart selects aligned to the left isn't the best idea for a narrow screen! Up until now I've only been using main UI from a desktop

This has been a learning experience for me. I will try to amend this.

  • the native back button navigation from the phone (on the bottom of all screens) often caused main UI to break/freeze. This is occurring in a few places in main UI (eg item editor in dirty situation). I'm still not familiar with f7's route and path history. The same issue happens on desktop and it has been a long time annoyance for me but this is for a separate topic.

@ghys
Copy link
Member

ghys commented Feb 24, 2024

Tbh I've never tried accessing the main UI from a mobile phone

This is indeed a problem and something that you MUST do before altering designs.
You just have to test your changes with the 3 themes, both dark & light, and on various devices before you submit impactful changes.
We have 3 seats available on our sponsored BrowserStack team and I'm happy to give one to you as you seem to be serious about developing the UI. Just reach out with an email address in private on the forum.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request main ui Main UI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

The addon store sub-categories
3 participants