Gui: Submenu and VarItemList API additions and improvements #3621

Willy-JL · 2024-04-30T05:30:35Z

What's new

General improvements:

Can have header in Variable Item List
Can have a vertical Submenu
Can lock items in Variable Item List and Submenu
Variable Item List label and value text will scroll if too long
Variable Item List value arrows shrink if value is < 4 characters
More freedom and usefulness in Variable Item List API
Cleaned up some parts of the code and some magic numbers are now defined
Feature parity between submenu and variable item list, only differences now are that:
- variable item list item ID is based on index of addition, submenu item ID is based on user-supplied 32bit value
- variable item list can edit item values, submenu only allows picking one (by design of the widget purpose)
- variable item list items can be edited after creation, submenu items cannot (by design of the widget purpose)

API additions (no removals, fully backwards compatible):

submenu_add_lockable_item():
- based on Add lockable capability to Submenu module #2289 which was rejected, CFWs merged it and improved it over time
- theres a few apps that use it by now
- example usecase: highlighting that some features are not available in current configuration / state
- and even more useful in varitemlist version below
submenu_set_orientation():
- can have vertical submenu (user's preference for ux)
- same concept works great in infrared app, could be reused there to make more of the app vertical instead of having to keep switching
variable_item_list_get():
- get and allow editing an item after creation
- example usecase: editing one option changes state of other entries in the list, like if they are locked due to incompatible settings or if they have different labels or values
- Sub-GHz app is already doing this (though in a very hacky way) here, to change frequency text to --- when hopping is enabled, by storing the item pointer in scene state, this is also quite error prone, with varitemlist get() it would be safer and clearer
- for example i have a an app menu that edits an array, you select an index by scrolling on first item, second item removes that index when clicked, third item adds a new item when clicked, this needs to edit the first item to show what index is selected after remove/add
variable_item_list_set_header():
- feature parity with submenu widget
- example usecase: headers are pretty but want editable options not just a list
variable_item_set_item_label():
- allow changing label after adding item
- example usecase: want to show index of item, can have value shown and put (1/10) in label dynamically, like in my example above in get()
- more too probably, allows more creativity
variable_item_set_locked():
- same as above for submenu, based on same code but ported to varitemlist by me
- also allows setting locked state after the fact, remembers locked message so after creation you can toggle with set_locked(item, false/true, NULL)
- example usecase: some settings combinations in this screen are incompatible, like GPIO pins overlapping when configuring

Acknowlegments:

Original submenu lockable items code by @giacomoferretti in Add lockable capability to Submenu module #2289
Vertical submenu and some code improvements by Unleashed CFW team
Most other edits by me @Willy-JL
Hope I'm not forgetting someone else that was involved

Verification

Check apps that use variable item list and submenu to be working correctly
No dedicated example app for additional APIs but some apps do use them, for example:
- Magspoof is using both submenu/varitemlist lockable items and varitemlist get() to dynamically edit a config menu based on allowed combinations of GPIO pins
- NFC Maker is using locked submenu items when choosing NTAG card type to show if payload is too big for some types
- XRemote is using submenu vertical orientation in a similar fashion to infrared app
- FindMy Flipper is using varitemlist header in its tag setup pages
- BLE Spam is using varitemlist get() to edit which subarray is shown, one option chooses a type and next option chooses allowed values for that type, editing first one needs to update second one
- An edited version of Mass Storage uses varitemlist header in disk image creation screen which allows setting image name and size
- Some CFWs show hidden debug options as locked so the user knows they are available but they probably should not use them, keeping them secret could be detrimental to some advanced user who might need them (for example: lfrfid read raw)

Checklist (For Reviewer)

PR has description of feature/bug or link to Confluence/Jira task
Description contains actions to verify feature/bugfix
I've built this code, uploaded it to the device and verified feature/bugfix

@giacomoferretti

API additions: - `submenu_add_lockable_item()`: based on flipperdevices#2289 which was rejected, CFWs merged it and improved it over time, theres a few apps that use it by now (example usecase: highlighting that some features are not available in current configuration / state) - `submenu_set_orientation()`: can have vertical submenu (user's preference for ux, same concept works great in infrared app, could be reused there to make more of the app vertical instead of having to keep switching) - `variable_item_list_get()`: get and allow editing an item after creation (example usecase: editing one option changes state of other entries in the list, like if they are locked due to incompatible settings or if they have different labels or values) - `variable_item_list_set_header()`: feature parity with `submenu` widget (example usecase: headers are pretty but want editable options not just a list) - `variable_item_set_item_label()`: allow changing label after adding item (example usecase: want to show index of item, can have value shown and put (1/10) in label dynamically, also probably more, allows more creativity) - `variable_item_set_locked()`: same as above for `submenu`, based on same code but ported to varitemlist by me. also allows setting locked state after the fact, remembers locked message so after creation you can toggle with `set_locked(item, false/true, NULL)` (example usecase, some settings combinations in this screen are incompatible, like GPIO pins overlapping when configuring) General improvements: - Can have header in Variable Item List - Can have a vertical Submenu - Can lock items in Variable Item List and Submenu - Variable Item List label and value scroll if too long - Variable Item List value arrows shrink if value is < 4 characters - More freedom and usefulness in Variable Item List API - Cleaned up some parts of the code and some magic numbers are now defined Acknowlegments: - Original submenu lockable items code by @giacomoferretti in flipperdevices#2289 - Vertical submenu and some code improvements by Unleashed CFW team - Most other edits by me @Willy-JL - Hope I'm not forgetting someone else that was involved

Willy-JL · 2024-04-30T05:30:57Z

This is in response to developer frustration with inconsistent APIs first-hand from @zacharyweiss when recently updating his Magspoof app, as well as discussion with @skotopes about differences between forks' APIs. As the lead maintainer of one such fork I've developed a decent amount for Flipper in the last year, some of which resides in additional APIs which I deemed useful. My past with the great people of OFW has been rocky to say the least but I wish to help the community and this wonderful project in whatever way I can, and while backporting all of the custom features would mean my fork wouldn't have a purpose anymore, I think (and aku seems to agree) that atleast having a consistent API would be a great start. This is some of that, but I'm planning to go through the rest too. One step at a time )

That being said, I'm aware of the comments made for example in #2289 about wanting a small and simple toolkit, and some (if not all) the changes in this PR would go against that wish. I simply ported and cleaned up the most developed submenu and varitemlist implementations that I'm aware of, then we can discuss here which parts (if any) would fit OFW's philosophy.

I also just realized that there's a few more things that could be added, namely:

a way to remove items from varitemlist, this would allow full control without reset() and adding all the items back
same goes for submenu to remove an item
having submenu scroll the text for long labels too

Willy-JL · 2024-05-03T09:28:01Z

I noticed that I had ported without the centering for varitemlist value text, so it was messed up. Added the extra elements APIs, one of which was needed for centered scrolling text in varitemlist value text.

Again, this is to port over APIs in hopes of making them consistent across forks, but I totally understand if you don't think these belong in OFW

…irmware into submenu-varitemlist-api

skotopes · 2024-05-16T16:11:09Z

Is it possible to split it into separate features?

I'm ready to merge some parts immediately and some I'd like to discuss more.

Willy-JL · 2024-05-16T20:20:44Z

@skotopes sure I can do that, which parts should I split?

skotopes · 2024-06-11T13:54:26Z

applications/services/gui/elements.c

+    FuriString* string,
+    size_t scroll,
+    bool ellipsis,
+    bool centered) {


How about calling it elements_scrollable_text_line_centered and instead of bool centered accept Align?

skotopes · 2024-06-11T13:54:51Z

applications/services/gui/elements.c

        if(ellipsis) {
            width -= canvas_string_width(canvas, "...");
        }

        // Calculate scroll size
        size_t scroll_size = furi_string_size(line);
        size_t right_width = 0;
-        for(size_t i = scroll_size; i > 0; i--) {
+        for(size_t i = scroll_size - 1; i > 0; i--) {


and if line is empty?

skotopes · 2024-06-11T14:01:44Z

applications/services/gui/modules/submenu.c

+    submenu_add_lockable_item(submenu, label, index, callback, callback_context, false, NULL);
+}
+
+void submenu_add_lockable_item(


Why not to have setter to lock item? in that way there will be no need for special constructor?

skotopes · 2024-06-11T14:06:13Z

applications/services/gui/modules/submenu.c

@@ -391,3 +490,45 @@ void submenu_set_header(Submenu* submenu, const char* header) {
        },
        true);
 }
+
+void submenu_set_orientation(Submenu* submenu, ViewOrientation orientation) {


Normally orientation is controlled by underlying view. Basically you can change view orientation and then use canvas info to match orientation.

skotopes · 2024-06-11T15:08:52Z

Sorry for keeping you waiting. I've left some comments regarding APIs.

Other than that I'd like to see this PR being split:

Elements
SubMenu
Variable item list

Willy-JL added 2 commits April 30, 2024 05:31

Fix ellipsis

3a1a539

Willy-JL requested review from skotopes, DrZlo13, hedger, gsurkov and nminaylov as code owners April 30, 2024 05:30

Willy-JL added 4 commits May 3, 2024 10:20

Gui: Elements centered scrollable text and horizontal scrollbar

20cb434

Gui: Fix varitemlist value text position

ba8b8e3

Update symbols

add64ef

Fix typo

3237c24

Gui: Fix scrolling text when only 1 char over limit

942f78a

hedger added UI Affects UI New Feature Contains an IMPLEMENTATION of a new feature Core+Services HAL, furi & core system services labels May 8, 2024

Willy-JL and others added 4 commits May 15, 2024 06:09

Merge branch 'dev' of https://github.com/flipperdevices/flipperzero-f…

d0c4d0c

…irmware into submenu-varitemlist-api

Merge branch 'dev' into submenu-varitemlist-api

fadef34

Merge branch 'dev' into submenu-varitemlist-api

5624c5f

Bump API symbols version

974cd38

Merge remote-tracking branch 'origin/dev' into submenu-varitemlist-api

1b88828

skotopes requested changes Jun 11, 2024

View reviewed changes

skotopes marked this pull request as draft June 11, 2024 15:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Gui: Submenu and VarItemList API additions and improvements #3621

Gui: Submenu and VarItemList API additions and improvements #3621

Willy-JL commented Apr 30, 2024

Willy-JL commented Apr 30, 2024 •

edited

Loading

Willy-JL commented May 3, 2024

skotopes commented May 16, 2024

Willy-JL commented May 16, 2024

skotopes Jun 11, 2024

skotopes Jun 11, 2024

skotopes Jun 11, 2024

skotopes Jun 11, 2024

skotopes commented Jun 11, 2024

Gui: Submenu and VarItemList API additions and improvements #3621

Are you sure you want to change the base?

Gui: Submenu and VarItemList API additions and improvements #3621

Conversation

Willy-JL commented Apr 30, 2024

What's new

Verification

Checklist (For Reviewer)

Willy-JL commented Apr 30, 2024 • edited Loading

Willy-JL commented May 3, 2024

skotopes commented May 16, 2024

Willy-JL commented May 16, 2024

skotopes Jun 11, 2024

Choose a reason for hiding this comment

skotopes Jun 11, 2024

Choose a reason for hiding this comment

skotopes Jun 11, 2024

Choose a reason for hiding this comment

skotopes Jun 11, 2024

Choose a reason for hiding this comment

skotopes commented Jun 11, 2024

Willy-JL commented Apr 30, 2024 •

edited

Loading