-
Notifications
You must be signed in to change notification settings - Fork 1.3k
App Loader: Add dropdowns for sorting and filtering #4150
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
base: master
Are you sure you want to change the base?
Conversation
|
Very nice! |
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.
Pull request overview
This PR replaces the chip-based filtering and sorting UI with dropdown menus in the App Loader, aiming to reduce visual clutter especially on mobile devices. The changes introduce three new dropdowns for device type selection, app filtering, and sorting options, along with a relocated search form.
Changes:
- Replaced chip-based filter/sort controls with dropdown menus wrapped in a styled "pill" container
- Reorganized the search form to be inline with the dropdown controls
- Added CSS styling for dropdown menus, including positioning, z-index management, and the dropdowns-pill container
- Commented out the old chip-based UI implementation
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 14 comments.
| File | Description |
|---|---|
| index.html | Restructured the app library interface to use three dropdown menus (device type, filter, sort) instead of chips, moved search form into the dropdown container, and commented out old chip UI |
| css/main.css | Added styling for dropdown menus, menu items, dropdowns-pill container, search form flexbox layout, and dropdown positioning with z-index controls |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| .inline-icon { | ||
| display: inline-block; /* Ensures the image behaves inline but allows width/height adjustments */ | ||
| width: 1em; /* Set the desired width */ | ||
| height: 1em; /* Set the desired height */ | ||
| vertical-align:sub; | ||
| margin-right: 5px; | ||
| } |
Copilot
AI
Feb 3, 2026
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.
The .inline-icon class is defined but not used anywhere in the codebase (checked in HTML and JS files). If this class is not needed for the dropdown functionality, consider removing it to reduce code clutter. If it's intended for future use or is used in external submodules, add a comment explaining its purpose.
| .inline-icon { | |
| display: inline-block; /* Ensures the image behaves inline but allows width/height adjustments */ | |
| width: 1em; /* Set the desired width */ | |
| height: 1em; /* Set the desired height */ | |
| vertical-align:sub; | |
| margin-right: 5px; | |
| } |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
I think we need to wait for espruino/EspruinoAppLoaderCore#87 to go in before we merge this? As we're just putting this in, please could you change ... also the EspruinoAppLoaderCore pr still needs the indentation changing and that |
| @@ -96,17 +145,9 @@ | |||
| <label class="chip hidden tooltip" sortid="installs" data-tooltip="Most installed by users">Installed</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.
We should really remove this rather than just commenting it out
| <!-- menu component --> | ||
| <ul class="menu"> | ||
| <li class="menu-item"><a sortid="">None</a></li> | ||
| <li class="menu-item"><a sortid="created">New</a></li> |
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.
The idea of these is that New/Updated are hidden until we actually have the data to make them work - in the one you commented out <label class="chip hidden tooltip" sortid="created" data-tooltip="Most recent apps">New</label>you can see it says hidden
But at least in your app loader (https://rkboss6.github.io/BangleApps) this is now broken - appdates.csv can't be found but the menu items are still shown anyway, they just don't work.
I'm also a bit concerned that we had tooltips on all the items before so that people could hover and see what they actually meant - but you've deleted those?
This stems from the discussion at #7920, where I had asked about having dropdowns for sorting and filtering, rather than a bunch of chips before. They got pretty messy with such a huge load of them on the screen, particularly for phones, so I made dropdowns instead. It's up on my app loader, and I have screenshots below. Any suggestions/feedback is very appreciated, check it out!
There is a JS component in appLoaderCore, which I'll try to get a PR for soon! Until then, I'm hoping we can discuss to make something everyone enjoys using.
Screenshots:
Before:
After: