-
-
Notifications
You must be signed in to change notification settings - Fork 898
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 support for multiple presets repositories to be active at once #3476
Conversation
This comment has been minimized.
This comment has been minimized.
AUTOMERGE: (FAIL)
|
Co-authored-by: Mark Haslinghuis <[email protected]>
@limonspb can you test this :) |
This comment has been minimized.
This comment has been minimized.
Thanks for comments and suggestions @limonspb!
|
thanks for doing that! i was procrastinating about this task for like a year now already lol |
Hey @limonspb Here's a screenshot of current version (build from the latest commit in this branch):
|
This comment has been minimized.
This comment has been minimized.
Awesome stuff, love the extra row with the source. Is it possible to hide it when ONLY betaflight official source is selected? And was thinking to have BF icon at the bottom right corner, because some preset names are long, we probably don't want to cut them even more. What do you think? Cutting keywords shorter is ok, only a few people pay attention to them anyways :) |
Hey @limonspb , cool idea regarding not showing it to ppl that have only official source selected. I've added that. Regarding the icon - I've tried both and I kinda like it top-right a bit more - I like having these two icons together rather than scattered around. Also given that background of that icon is transparent, it shows most of the text it's overlayed over (see screenshot). There's not that many presets with name that long, so it's not something that would affect majority of the people - but as backend dev I know very little about UX, so your call :-)))) |
This comment has been minimized.
This comment has been minimized.
Thanks a lot, @limonspb , I really appreciate the feedback! :) I've fixed the code duplication, so let me know if there're more comments. |
This comment has been minimized.
This comment has been minimized.
src/tabs/presets/presets.js
Outdated
@@ -117,7 +118,9 @@ presets.onSaveClick = function() { | |||
|
|||
presets.markPickedPresetsAsFavorites = function() { | |||
for(const pickedPreset of this.pickedPresetList) { |
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.
👾
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.
fixed 👍
Yes can you try how that looks - but trimming length could be an option? |
Kudos, SonarCloud Quality Gate passed! |
I've tried having icons on the left and it looks weird to me (sorry...) - given that you can have mix of the non-BF and BF presets in one row / column, presets' title starts jumping around based on the amount of icons it's being prefixed with ... I've implemented title trimming as suggested |
i dont want to add work nor confusion, but an option could also be usage of sub-tabs much like the PIDS/Filters. |
Do you want to test this code? Here you have an automated build: |
@nerdCopter I was thinking about similar approach when I started working on this, but what made me stay with current visual implementation was filtering of the presets - when you'd looking for e.g. HDZero stuff, you have it aggregated in one page, compared to multiple tabs when we'd need to click through separate tabs per repo to find all filtered data. I find it easier to have everything aggregated together (and distinguished by source repo tags / icon(s)) rather than forcing user click through 2-3 tabs to check if some of their favorite repos have some presets. |
yeah i agree that multiple tabs is not as nice. Better if search works in one place i think. Especially now when we have fancy algorithm to remove duplicates across multiple repos. |
So you’ve trimed the titles? It didn’t look good having them over the logos. You could put it under the star other wise that would look cleaner |
Or go through and edit the offenders to have a shorter title |
Yes, i'm trimming titles of presets, so icons (both logo and star) and text can no longer overlap. |
We can set a character limit on titles |
@@ -20,7 +20,7 @@ | |||
overflow: hidden; | |||
white-space: nowrap; | |||
text-overflow: ellipsis; | |||
width: calc(100% - 30px); | |||
width: calc(100% - 60px); |
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.
@sugaarK this (I guess) trims the length of the title.
But agree we should set a limit in the preset itself.
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.
Anyway this should be ready to go
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.
- seems to function as intended; i did not do deep-testing
Description
This adds support for multiple preset repositories to be active at once.
Testing done
Locally with a few remote and local repos
Links
Resolves #3437