Add support for multiple presets repositories to be active at once #3476

Benky · 2023-06-08T17:36:01Z

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

src/tabs/presets/presets.js

blckmn · 2023-06-08T19:17:04Z

AUTOMERGE: (FAIL)

github identifies PR as mergeable -> FAIL
assigned to a milestone -> PASS
cooling off period lapsed -> PASS
commit count less or equal to three -> FAIL
Don't merge label NOT found -> PASS
at least one RN: label found -> PASS
Tested label found -> FAIL
assigned to an approver -> FAIL
approver count at least three -> FAIL

Co-authored-by: Mark Haslinghuis <mark@numloq.nl>

haslinghuis · 2023-06-08T20:21:39Z

@limonspb can you test this :)

limonspb · 2023-06-09T01:45:05Z

This is awesome, thank you!
A couple of notes:

when multiple sources are selected, i think it still needs a warning (at the top of presets) that an unofficial source is active.
Some preset source authors leave all official presets in the custom repos. That makes them show twice now. So now the "favorite presets functionality" must also remember, which preset source it was picked from. And don't show preset as a favorite for example, if it was picked from the official repo, but the current preset bar is from, let's say Chris Rosser's repo.
Also now i think when multiple sources are selected, the presets from official repo need a special icon indicating its from the official repo. For example like that:
When one of the preset sources failed to load, i think, it still needs to show the other presets from other repos. And, probably, a warning "following preset sources are failed to load: ....."
In theory can also hide prest duplicates if the hash matches with official repo. Index file has hashes for each preset.

Benky · 2023-06-09T02:23:33Z

Thanks for comments and suggestions @limonspb!

- 1 - OK, that one should be easy 👍
- 3 - OK, I'll do that - good point! Do you want that logo to be in the bottom right or could I add it next to "favorite" icon, so we have all icons grouped together?
- 4 - OK, I'll pop a message and log a warning about that - I was thinking about that, but kinda forgot to do that :-D
- 5 & 2 - Thanks for mentioning these hashes - I can implement that deduplication, but thinking about the design a bit more, I think adding name of the preset source to these tiles (PresetTitlePanelBody) might help people to identify where each preset comes from (if that's important for them). I'll also modify "favorite presets functionality" as you're describing 👍

limonspb · 2023-06-09T02:28:18Z

Thanks for comments and suggestions @limonspb!

1 - OK, that one should be easy 👍

3 - OK, I'll do that - good point! Do you want that logo to be in the bottom right or could I add it next to "favorite" icon, so we have all icons grouped together?

4 - OK, I'll pop a message and log a warning about that - I was thinking about that, but kinda forgot to do that :-D

5 & 2 - Thanks for mentioning these hashes - I can implement that deduplication, but thinking about the design a bit more, I think adding name of the preset source to these tiles (PresetTitlePanelBody) might help people to identify where each preset comes from (if that's important for them). I'll also modify "favorite presets functionality" as you're describing 👍

thanks for doing that! i was procrastinating about this task for like a year now already lol

Benky · 2023-06-09T15:44:48Z

Hey @limonspb

Here's a screenshot of current version (build from the latest commit in this branch):

warning for 3rd party repo should be visible when there's at least one 3rd party repo selected (visible on screenshot)
I've implemented preset deduplication based on hash. We now store additional information in localStorage to be able to distinguish the same preset in different repos.
new icon to distinguish official presets from 3rd party ones added (visible on screenshot)
when repository fails to load, it's name is reported in the main screen (visible on screenshot)
there's a new row in preset overview (and detail) showing name of the repository the preset comes from

limonspb · 2023-06-10T08:25:36Z

Hey @limonspb

Here's a screenshot of current version (build from the latest commit in this branch):

warning for 3rd party repo should be visible when there's at least one 3rd party repo selected (visible on screenshot)

I've implemented preset deduplication based on hash. We now store additional information in localStorage to be able to distinguish the same preset in different repos.

new icon to distinguish official presets from 3rd party ones added (visible on screenshot)

when repository fails to load, it's name is reported in the main screen (visible on screenshot)

there's a new row in preset overview (and detail) showing name of the repository the preset comes from

Awesome stuff, love the extra row with the source. Is it possible to hide it when ONLY betaflight official source is selected?
The majority of people, i assume, use only Betaflight repo. They don't need to see that its betaflight repo, unless they start messing with sources.

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 :)

Benky · 2023-06-10T14:47:04Z

Hey @limonspb
Here's a screenshot of current version (build from the latest commit in this branch):

warning for 3rd party repo should be visible when there's at least one 3rd party repo selected (visible on screenshot)

I've implemented preset deduplication based on hash. We now store additional information in localStorage to be able to distinguish the same preset in different repos.

new icon to distinguish official presets from 3rd party ones added (visible on screenshot)

when repository fails to load, it's name is reported in the main screen (visible on screenshot)

there's a new row in preset overview (and detail) showing name of the repository the preset comes from

Awesome stuff, love the extra row with the source. Is it possible to hide it when ONLY betaflight official source is selected? The majority of people, i assume, use only Betaflight repo. They don't need to see that its betaflight repo, unless they start messing with sources.

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 :-))))

limonspb · 2023-06-11T01:25:46Z

you are the designer here ha :)
So if you think this one is better, let it be :)

A few notes about the code, much appreciated supporting the clean-small-function style, easy to read.

This new section is being repeated in two files:

I think you can just move it into "PresetsRepoIndexed.js", because GithubRepo and WebsiteRepo both inherit from there.
So its actually just one note :)
Maybe someone can add anything else

Benky · 2023-06-11T02:52:39Z

Thanks a lot, @limonspb , I really appreciate the feedback! :)

I've fixed the code duplication, so let me know if there're more comments.

haslinghuis · 2023-06-12T22:59:01Z

src/tabs/presets/presets.js

@@ -117,7 +118,9 @@ presets.onSaveClick = function() {

 presets.markPickedPresetsAsFavorites = function() {
    for(const pickedPreset of this.pickedPresetList) {


haslinghuis · 2023-06-13T23:50:02Z

Pulling the icons to the left side might be cleaner || limit title length.

Benky · 2023-06-14T03:34:10Z

Pulling the icons to the left side might be cleaner || limit title length.

by "to the left side" you mean before the title itself?

haslinghuis · 2023-06-14T09:23:08Z

Yes can you try how that looks - but trimming length could be an option?

sonarcloud · 2023-06-14T19:26:02Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
No Duplication information

Benky · 2023-06-14T19:26:08Z

Yes can you try how that looks - but trimming length could be an option?

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

nerdCopter · 2023-06-14T19:35:43Z

i dont want to add work nor confusion, but an option could also be usage of sub-tabs much like the PIDS/Filters.
i.e. Official presets on a sub-tab. source 2 presets on a sub-tab, source 3 presets on another tab....

github-actions · 2023-06-14T19:38:37Z

Do you want to test this code? Here you have an automated build:
Betaflight-Configurator-Android
Betaflight-Configurator-Linux
Betaflight-Configurator-macOS
Betaflight-Configurator-Windows
WARNING: It may be unstable and result in corrupted configurations or data loss. Use only for testing!

Benky · 2023-06-14T21:54:56Z

@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.

limonspb · 2023-06-15T11:25:57Z

@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.

sugaarK · 2023-06-17T05:09:16Z

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

sugaarK · 2023-06-17T05:10:37Z

Or go through and edit the offenders to have a shorter title

Benky · 2023-06-17T13:03:52Z

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

Yes, i'm trimming titles of presets, so icons (both logo and star) and text can no longer overlap.

sugaarK · 2023-06-17T23:31:17Z

We can set a character limit on titles

haslinghuis · 2023-06-18T00:32:08Z

src/tabs/presets/TitlePanel/PresetTitlePanel.css

@@ -20,7 +20,7 @@
    overflow: hidden;
    white-space: nowrap;
    text-overflow: ellipsis;
-    width: calc(100% - 30px);
+    width: calc(100% - 60px);


@sugaarK this (I guess) trims the length of the title.

But agree we should set a limit in the preset itself.

Anyway this should be ready to go

nerdCopter

seems to function as intended; i did not do deep-testing

Add support for multiple presets repositories to be active at once

da3b9ad

This comment has been minimized.

Sign in to view

haslinghuis reviewed Jun 8, 2023

View reviewed changes

src/tabs/presets/presets.js Outdated Show resolved Hide resolved

haslinghuis assigned Benky Jun 8, 2023

haslinghuis added the RN: IMPROVEMENT label Jun 8, 2023

haslinghuis added this to the 10.10.0 milestone Jun 8, 2023

Code-style changes to src/tabs/presets/presets.js

67a69f9

Co-authored-by: Mark Haslinghuis <mark@numloq.nl>

This comment has been minimized.

Sign in to view

Code-review changes

1f98814

This comment has been minimized.

Sign in to view

Show presets repo name only when non-official repos are enabled

846d210

This comment has been minimized.

Sign in to view

Code-review changes

346f5a8

This comment has been minimized.

Sign in to view

limonspb approved these changes Jun 12, 2023

View reviewed changes

haslinghuis reviewed Jun 12, 2023

View reviewed changes

haslinghuis approved these changes Jun 12, 2023

View reviewed changes

haslinghuis requested review from McGiverGim and nerdCopter June 13, 2023 21:56

Trim preset title not to overlap icons

41ff12e

haslinghuis reviewed Jun 18, 2023

View reviewed changes

haslinghuis added the Ready to Merge label Jun 18, 2023

nerdCopter approved these changes Jun 21, 2023

View reviewed changes

haslinghuis merged commit ad9b29e into betaflight:master Jun 21, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for multiple presets repositories to be active at once #3476

Add support for multiple presets repositories to be active at once #3476

Benky commented Jun 8, 2023 •

edited by nerdCopter

Loading

This comment has been minimized.

blckmn commented Jun 8, 2023 •

edited

Loading

haslinghuis commented Jun 8, 2023

This comment has been minimized.

limonspb commented Jun 9, 2023 •

edited

Loading

Benky commented Jun 9, 2023 •

edited

Loading

limonspb commented Jun 9, 2023

Benky commented Jun 9, 2023 •

edited

Loading

This comment has been minimized.

limonspb commented Jun 10, 2023 •

edited

Loading

Benky commented Jun 10, 2023

This comment has been minimized.

limonspb commented Jun 11, 2023 •

edited

Loading

Benky commented Jun 11, 2023 •

edited

Loading

This comment has been minimized.

haslinghuis Jun 12, 2023

Benky Jun 14, 2023

haslinghuis commented Jun 13, 2023 •

edited

Loading

Benky commented Jun 14, 2023

haslinghuis commented Jun 14, 2023 •

edited

Loading

sonarcloud bot commented Jun 14, 2023

Benky commented Jun 14, 2023

nerdCopter commented Jun 14, 2023

github-actions bot commented Jun 14, 2023

Benky commented Jun 14, 2023

limonspb commented Jun 15, 2023 •

edited

Loading

sugaarK commented Jun 17, 2023

sugaarK commented Jun 17, 2023

Benky commented Jun 17, 2023

sugaarK commented Jun 17, 2023

haslinghuis Jun 18, 2023

haslinghuis Jun 18, 2023

nerdCopter left a comment

		@@ -117,7 +118,9 @@ presets.onSaveClick = function() {

		presets.markPickedPresetsAsFavorites = function() {
		for(const pickedPreset of this.pickedPresetList) {

Add support for multiple presets repositories to be active at once #3476

Add support for multiple presets repositories to be active at once #3476

Conversation

Benky commented Jun 8, 2023 • edited by nerdCopter Loading

Description

Testing done

Links

This comment has been minimized.

blckmn commented Jun 8, 2023 • edited Loading

haslinghuis commented Jun 8, 2023

This comment has been minimized.

limonspb commented Jun 9, 2023 • edited Loading

Benky commented Jun 9, 2023 • edited Loading

limonspb commented Jun 9, 2023

Benky commented Jun 9, 2023 • edited Loading

This comment has been minimized.

limonspb commented Jun 10, 2023 • edited Loading

Benky commented Jun 10, 2023

This comment has been minimized.

limonspb commented Jun 11, 2023 • edited Loading

Benky commented Jun 11, 2023 • edited Loading

This comment has been minimized.

haslinghuis Jun 12, 2023

Choose a reason for hiding this comment

Benky Jun 14, 2023

Choose a reason for hiding this comment

haslinghuis commented Jun 13, 2023 • edited Loading

Benky commented Jun 14, 2023

haslinghuis commented Jun 14, 2023 • edited Loading

sonarcloud bot commented Jun 14, 2023

Benky commented Jun 14, 2023

nerdCopter commented Jun 14, 2023

github-actions bot commented Jun 14, 2023

Benky commented Jun 14, 2023

limonspb commented Jun 15, 2023 • edited Loading

sugaarK commented Jun 17, 2023

sugaarK commented Jun 17, 2023

Benky commented Jun 17, 2023

sugaarK commented Jun 17, 2023

haslinghuis Jun 18, 2023

Choose a reason for hiding this comment

haslinghuis Jun 18, 2023

Choose a reason for hiding this comment

nerdCopter left a comment

Choose a reason for hiding this comment

Benky commented Jun 8, 2023 •

edited by nerdCopter

Loading

blckmn commented Jun 8, 2023 •

edited

Loading

limonspb commented Jun 9, 2023 •

edited

Loading

Benky commented Jun 9, 2023 •

edited

Loading

Benky commented Jun 9, 2023 •

edited

Loading

limonspb commented Jun 10, 2023 •

edited

Loading

limonspb commented Jun 11, 2023 •

edited

Loading

Benky commented Jun 11, 2023 •

edited

Loading

haslinghuis commented Jun 13, 2023 •

edited

Loading

haslinghuis commented Jun 14, 2023 •

edited

Loading

limonspb commented Jun 15, 2023 •

edited

Loading