Add increment and decrement controls for numeric display options #745

ahmedaabouzied · 2021-08-16T05:48:39Z

Display controls to increment & decrement the numeric options.
Display control to select/deselect non-numeric options with the enter key.
This allows incrementing and decrementing values with the mouse.

Resolves: #737

Screenshot after applying the change:

DisplayOptionsPanel.c

Explorer09 · 2021-08-17T00:53:56Z

The increment button should be on the right and the decrement on the left.
Example picture from Nielsen Norman Group
There's a UI design guideline for such a numeric control.
Hide the increment and decrement buttons on non-numeric controls. It's more confusing when a boolean control is highlighted and user tries increment and decrement buttons and found out they don't work.
I don't like the "(De)Select" shorthand. Just label it "Select".

ahmedaabouzied · 2021-08-24T15:31:22Z

I've updated the PR in response to @Explorer09 comments with the following changes:

Renames the (De)select button name to Select.
Makes the Increment button on the right of the Decrement button.
Shows two different function bars according to the type of item selected.
- It will show a numeric function bar with Decrement and Increment buttons with numeric items.
- It will show a boolean function bar with the Select button with boolean items.

DisplayOptionsPanel.c

ahmedaabouzied · 2021-08-24T16:22:38Z

@BenBE I think my editor is messing up the indentation. Is there a specific linter I should run?

BenBE · 2021-08-24T16:30:28Z

@BenBE I think my editor is messing up the indentation. Is there a specific linter I should run?

There is a rough command line mentioned in the STYLEGUIDE using astyle. Be sure to only include the lines near the code you edited.

ahmedaabouzied · 2021-08-24T16:33:52Z

There is a rough command line mentioned in the STYLEGUIDE using astyle. Be sure to only include the lines near the code you edited.

@BenBE , I think just manually fixed the indentation issues here. Could you take another look?

BenBE · 2021-08-24T16:35:31Z

There is a rough command line mentioned in the STYLEGUIDE using astyle. Be sure to only include the lines near the code you edited.

@BenBE , I think just manually fixed the indentation issues here. Could you take another look?

Quick look shows still some issues left. Check for 3 spaces per level.

ahmedaabouzied · 2021-08-24T16:40:29Z

Quick look shows still some issues left. Check for 3 spaces per level.

Ok, 3 spaces it is.

BenBE · 2021-08-24T16:42:54Z

Another small adjustment:

$ git diff --cached 
diff --git a/DisplayOptionsPanel.c b/DisplayOptionsPanel.c
index 42489077..176b017e 100644
--- a/DisplayOptionsPanel.c
+++ b/DisplayOptionsPanel.c
@@ -27,8 +27,6 @@ static const char* const NumericDisplayOptionsFunctions[] = {"Decrement ", "Incr
 static const char* const NumericDisplayOptionsKeys[] = {"-", "+", "F10", "Esc"};
 static const int NumericDisplayOptionsEvents[] = {'-', '+', KEY_F(10), 27};
 
-
-
 static void DisplayOptionsPanel_delete(Object* object) {
    Panel* super = (Panel*) object;
    DisplayOptionsPanel* this = (DisplayOptionsPanel*) object;
@@ -54,15 +52,15 @@ static HandlerResult DisplayOptionsPanel_eventHandler(Panel* super, int ch) {
 
    switch (ch) {
 
-   case KEY_UP:{
+   case KEY_UP: {
       int selected_index = Panel_getSelectedIndex(super);
       if (selected_index > 1) {
          OptionItem* next_to_select = (OptionItem*) Panel_get(super, selected_index -1);
          DisplayOptionsPanel_setFunctionBar(super, next_to_select);
       }
       break;
-    }
-   case KEY_DOWN:{
+   }
+   case KEY_DOWN: {
       int selected_index = Panel_getSelectedIndex(super);
       if (selected_index < Panel_size(super) - 1) {
          OptionItem* next_to_select = (OptionItem*) Panel_get(super, selected_index +1);

ahmedaabouzied · 2021-08-24T17:13:03Z

@BenBE Alright.

ahmedaabouzied · 2021-08-24T17:16:20Z

I resolved to format the file with asyle as stated in the /doc/styleguide.md file which messed things even further. This is not fun XD

ahmedaabouzied · 2021-08-24T17:37:14Z

@BenBE , sorry for all that mess. I think now it's reasonably formatted.

BenBE

What about merging the two commits of this PR?

BenBE · 2021-08-24T17:46:50Z

DisplayOptionsPanel.c

@@ -90,7 +122,7 @@ const PanelClass DisplayOptionsPanel_class = {
 DisplayOptionsPanel* DisplayOptionsPanel_new(Settings* settings, ScreenManager* scr) {
   DisplayOptionsPanel* this = AllocThis(DisplayOptionsPanel);
   Panel* super = (Panel*) this;
-   FunctionBar* fuBar = FunctionBar_new(DisplayOptionsFunctions, NULL, NULL);
+   FunctionBar* fuBar = FunctionBar_new(BooleanDisplayOptionsFunctions, BooleanDisplayOptionsKeys, BooleanDisplayOptionsEvents);


What about creating the Panel without a Function Bar at first and just set it based on the default-selected item just before returning? Avoids having this statement go out of sync when the first item changes.

Also maybe rename Boolean* to Checkbox*?

Checkbox seems more reasonable @BenBE

Will update that

ahmedaabouzied · 2021-08-28T16:05:50Z

@BenBE , now initially the Panel is initiated with an empty function bar, then the right function bar is set based on the default selected item using the DisplayOptionsPanel_setFunctionBar() function. This will make it so that if that first item type is changed, the function bar will change with it.

Let me know if you have more comments.
Thanks.

BenBE · 2021-08-28T19:29:03Z

DisplayOptionsPanel.c

@@ -29,13 +34,41 @@ static void DisplayOptionsPanel_delete(Object* object) {
   free(this);
 }

+static void DisplayOptionsPanel_setFunctionBar(Panel* super, OptionItem* item) {


This leaks the memory of the previously selected FunctionBar.

Good catch @BenBE , modified that. Sorry for the delayed responses btw.

BenBE · 2021-10-15T13:50:53Z

Given there's #822 on the list of likely extensions, it's good planning to go the switch statement route directly.

BenBE

Only one additional point that @cgzones didn't remark on in his latest review …

DisplayOptionsPanel.c

BenBE · 2021-11-28T19:37:10Z

Hi @ahmedaabouzied, any updates with this PR? Need help with any of the issues raised?

Signed-off-by: Ahmed Abouzied <[email protected]>

ahmedaabouzied · 2021-11-30T10:02:32Z

Hi @ahmedaabouzied, any updates with this PR? Need help with any of the issues raised?

Sorry @BenBE , I missed the notification there. I've applied the changes you requested above.

cgzones · 2021-12-05T19:05:52Z

I think one can re-use the drawFunctionBar callback of the Panel class, which avoids the KEY_UP/DOWN interception:

diff --git a/DisplayOptionsPanel.c b/DisplayOptionsPanel.c
index 8212120..0e3ac03 100644
--- a/DisplayOptionsPanel.c
+++ b/DisplayOptionsPanel.c
@@ -20,11 +20,18 @@ in the source distribution for its full text.
 #include "ProvideCurses.h"
 
 
-static const char* const DisplayOptionsFunctions[] = {"      ", "      ", "      ", "      ", "      ", "      ", "      ", "      ", "      ", "Done  ", NULL};
+static const char* const CheckboxDisplayOptionsFunctions[] = {"Select  ", "Done  ", NULL};
+static const char* const CheckboxDisplayOptionsKeys[] = {"Enter", "F10"};
+static const int CheckboxDisplayOptionsEvents[] = {KEY_ENTER, KEY_F(10)};
+static const char* const NumericDisplayOptionsFunctions[] = {"Decrement ", "Increment ", "Done  ", NULL};
+static const char* const NumericDisplayOptionsKeys[] = {"-", "+", "F10"};
+static const int NumericDisplayOptionsEvents[] = {'-', '+', KEY_F(10)};
 
 static void DisplayOptionsPanel_delete(Object* object) {
    Panel* super = (Panel*) object;
    DisplayOptionsPanel* this = (DisplayOptionsPanel*) object;
+   FunctionBar_delete(this->numericFuBar);
+   FunctionBar_delete(this->checkboxFuBar);
    Panel_done(super);
    free(this);
 }
@@ -79,22 +86,46 @@ static HandlerResult DisplayOptionsPanel_eventHandler(Panel* super, int ch) {
    return result;
 }
 
+static void DisplayOptionsPanel_drawFunctionBar(Panel* super, ATTR_UNUSED bool hideFunctionBar) {
+   DisplayOptionsPanel* this = (DisplayOptionsPanel*) super;
+
+   int selected_index = Panel_getSelectedIndex(super);
+   OptionItem* item = (OptionItem*) Panel_get(super, selected_index);
+
+   switch (OptionItem_kind(item)) {
+      case OPTION_ITEM_NUMBER:
+         super->currentBar = this->numericFuBar;
+         break;
+      case OPTION_ITEM_CHECK:
+         super->currentBar = this->checkboxFuBar;
+         break;
+      default:
+         assert(0); // Unknown option type
+   }
+   FunctionBar_draw(super->currentBar);
+}
+
 const PanelClass DisplayOptionsPanel_class = {
    .super = {
       .extends = Class(Panel),
       .delete = DisplayOptionsPanel_delete
    },
-   .eventHandler = DisplayOptionsPanel_eventHandler
+   .eventHandler = DisplayOptionsPanel_eventHandler,
+   .drawFunctionBar = DisplayOptionsPanel_drawFunctionBar
 };
 
 DisplayOptionsPanel* DisplayOptionsPanel_new(Settings* settings, ScreenManager* scr) {
    DisplayOptionsPanel* this = AllocThis(DisplayOptionsPanel);
    Panel* super = (Panel*) this;
-   FunctionBar* fuBar = FunctionBar_new(DisplayOptionsFunctions, NULL, NULL);
-   Panel_init(super, 1, 1, 1, 1, Class(OptionItem), true, fuBar);
+   FunctionBar* checkboxFuBar = FunctionBar_new(CheckboxDisplayOptionsFunctions, CheckboxDisplayOptionsKeys, CheckboxDisplayOptionsEvents);
+   FunctionBar* numericFuBar = FunctionBar_new(NumericDisplayOptionsFunctions, NumericDisplayOptionsKeys, NumericDisplayOptionsEvents);
+   FunctionBar* emptyFuBar = FunctionBar_new(NULL, NULL, NULL);
+   Panel_init(super, 1, 1, 1, 1, Class(OptionItem), true, emptyFuBar);
 
    this->settings = settings;
    this->scr = scr;
+   this->numericFuBar = numericFuBar;
+   this->checkboxFuBar = checkboxFuBar;
 
    Panel_setHeader(super, "Display options");
    Panel_add(super, (Object*) CheckItem_newByRef("Tree view", &(settings->treeView)));
diff --git a/DisplayOptionsPanel.h b/DisplayOptionsPanel.h
index 5e87a63..899cf3c 100644
--- a/DisplayOptionsPanel.h
+++ b/DisplayOptionsPanel.h
@@ -17,6 +17,8 @@ typedef struct DisplayOptionsPanel_ {
 
    Settings* settings;
    ScreenManager* scr;
+   FunctionBar* numericFuBar;
+   FunctionBar* checkboxFuBar;
 } DisplayOptionsPanel;
 
 extern const PanelClass DisplayOptionsPanel_class;

macmbello · 2022-03-07T15:14:50Z

#the manage the upskill od the main menu
we have to atleat give the meaning of the menu's bar

BenBE · 2025-01-18T14:09:07Z

What's the status for this PR? Any open issues to resolve?

BenBE · 2025-01-18T14:11:43Z

DisplayOptionsPanel.c

+      case OPTION_ITEM_NUMBER: 
+         super->currentBar = this->numericFuBar;
+         break;
+      case OPTION_ITEM_CHECK: 
+         super->currentBar = this->checkboxFuBar;
+         break;
+      defaut:
+         assert(0); // Unknown option type 


Suggested change

case OPTION_ITEM_NUMBER:

super->currentBar = this->numericFuBar;

break;

case OPTION_ITEM_CHECK:

super->currentBar = this->checkboxFuBar;

break;

defaut:

assert(0); // Unknown option type

case OPTION_ITEM_NUMBER:

super->currentBar = this->numericFuBar;

break;

case OPTION_ITEM_CHECK:

super->currentBar = this->checkboxFuBar;

break;

defaut:

assert(0); // Unknown option type

@Explorer09

as per @Explorer09's suggestion in #745. Also makes it consistent with "- Nice" and "+ Nice" in the main function bar!

@Explorer09

as per @Explorer09's suggestion in #745. Also makes it consistent with "- Nice" and "+ Nice" in the main function bar!

…shamhm Closes #737, #745, #722

BenBE added the enhancement Extension or improvement to existing feature label Aug 16, 2021

BenBE added this to the 3.1.1 milestone Aug 16, 2021

BenBE reviewed Aug 16, 2021

View reviewed changes

DisplayOptionsPanel.c Outdated Show resolved Hide resolved

ahmedaabouzied force-pushed the issue-737 branch from f2f71b1 to e83c20a Compare August 16, 2021 07:28

BenBE approved these changes Aug 16, 2021

View reviewed changes

BenBE requested changes Aug 24, 2021

View reviewed changes

DisplayOptionsPanel.c Outdated Show resolved Hide resolved

DisplayOptionsPanel.c Outdated Show resolved Hide resolved

DisplayOptionsPanel.c Outdated Show resolved Hide resolved

DisplayOptionsPanel.c Outdated Show resolved Hide resolved

ahmedaabouzied force-pushed the issue-737 branch 3 times, most recently from cc0978b to 2994d36 Compare August 24, 2021 16:32

ahmedaabouzied force-pushed the issue-737 branch from 2994d36 to 1873434 Compare August 24, 2021 16:37

ahmedaabouzied force-pushed the issue-737 branch from 1873434 to 8153d84 Compare August 24, 2021 17:10

ahmedaabouzied force-pushed the issue-737 branch from 8153d84 to e1bf90f Compare August 24, 2021 17:14

ahmedaabouzied force-pushed the issue-737 branch from e1bf90f to ec93881 Compare August 24, 2021 17:35

BenBE reviewed Aug 24, 2021

View reviewed changes

ahmedaabouzied force-pushed the issue-737 branch from ec93881 to 9716bff Compare August 28, 2021 01:39

ahmedaabouzied requested a review from BenBE August 28, 2021 19:11

BenBE reviewed Aug 29, 2021

View reviewed changes

BenBE mentioned this pull request Sep 2, 2021

Show Function Bar as default on NULL #772

Draft

BenBE modified the milestones: 3.2.0, 3.1.2 Oct 17, 2021

BenBE requested changes Oct 28, 2021

View reviewed changes

DisplayOptionsPanel.c Outdated Show resolved Hide resolved

DisplayOptionsPanel.c Outdated Show resolved Hide resolved

ahmedaabouzied force-pushed the issue-737 branch from 76d88df to e87ed80 Compare October 30, 2021 16:19

ahmedaabouzied requested review from BenBE and cgzones October 30, 2021 16:22

ahmedaabouzied force-pushed the issue-737 branch 2 times, most recently from 6d7a5b9 to e1d6f9a Compare October 30, 2021 16:27

BenBE requested changes Oct 30, 2021

View reviewed changes

DisplayOptionsPanel.c Outdated Show resolved Hide resolved

BenBE modified the milestones: 3.1.2, 3.2.0 Nov 28, 2021

Add increment and decrement controls for numeric display options

a90a327

Signed-off-by: Ahmed Abouzied <[email protected]>

ahmedaabouzied force-pushed the issue-737 branch from e1d6f9a to a90a327 Compare November 30, 2021 10:00

fasterit modified the milestones: 3.2.0, 3.3.0 Feb 24, 2022

BenBE modified the milestones: 3.3.0, 3.4.0 Dec 26, 2023

BenBE reviewed Jan 18, 2025

View reviewed changes

BenBE modified the milestones: 3.4.0, 3.5.0 Jan 20, 2025

Explorer09 mentioned this pull request Feb 19, 2025

DisplayOptions: "Inc"/"Dec" operations in the function bar #1610

Merged

hishamhm added a commit that referenced this pull request Feb 19, 2025

DisplayOptions: flip function bar buttons to Dec and Inc

ffff5d2

as per @Explorer09's suggestion in #745. Also makes it consistent with "- Nice" and "+ Nice" in the main function bar!

hishamhm added a commit that referenced this pull request Feb 19, 2025

DisplayOptions: flip function bar buttons to Dec and Inc

4c6011c

as per @Explorer09's suggestion in #745. Also makes it consistent with "- Nice" and "+ Nice" in the main function bar!

fasterit pushed a commit that referenced this pull request Feb 20, 2025

Merge remote-tracking branch 'origin/display-options-inc-dec' from hi…

d28659d

…shamhm Closes #737, #745, #722

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add increment and decrement controls for numeric display options #745

Add increment and decrement controls for numeric display options #745

ahmedaabouzied commented Aug 16, 2021 •

edited

Loading

Explorer09 commented Aug 17, 2021

ahmedaabouzied commented Aug 24, 2021

ahmedaabouzied commented Aug 24, 2021

BenBE commented Aug 24, 2021

ahmedaabouzied commented Aug 24, 2021

BenBE commented Aug 24, 2021

ahmedaabouzied commented Aug 24, 2021

BenBE commented Aug 24, 2021

ahmedaabouzied commented Aug 24, 2021

ahmedaabouzied commented Aug 24, 2021 •

edited

Loading

ahmedaabouzied commented Aug 24, 2021

BenBE left a comment

BenBE Aug 24, 2021

ahmedaabouzied Aug 24, 2021

ahmedaabouzied Sep 12, 2021

ahmedaabouzied Sep 12, 2021

ahmedaabouzied commented Aug 28, 2021

BenBE Aug 28, 2021

ahmedaabouzied Sep 12, 2021

BenBE commented Oct 15, 2021

BenBE left a comment

BenBE commented Nov 28, 2021

ahmedaabouzied commented Nov 30, 2021

cgzones commented Dec 5, 2021

macmbello commented Mar 7, 2022

BenBE commented Jan 18, 2025

BenBE Jan 18, 2025

Add increment and decrement controls for numeric display options #745

Are you sure you want to change the base?

Add increment and decrement controls for numeric display options #745

Conversation

ahmedaabouzied commented Aug 16, 2021 • edited Loading

Explorer09 commented Aug 17, 2021

ahmedaabouzied commented Aug 24, 2021

ahmedaabouzied commented Aug 24, 2021

BenBE commented Aug 24, 2021

ahmedaabouzied commented Aug 24, 2021

BenBE commented Aug 24, 2021

ahmedaabouzied commented Aug 24, 2021

BenBE commented Aug 24, 2021

ahmedaabouzied commented Aug 24, 2021

ahmedaabouzied commented Aug 24, 2021 • edited Loading

ahmedaabouzied commented Aug 24, 2021

BenBE left a comment

Choose a reason for hiding this comment

BenBE Aug 24, 2021

Choose a reason for hiding this comment

ahmedaabouzied Aug 24, 2021

Choose a reason for hiding this comment

ahmedaabouzied Sep 12, 2021

Choose a reason for hiding this comment

ahmedaabouzied Sep 12, 2021

Choose a reason for hiding this comment

ahmedaabouzied commented Aug 28, 2021

BenBE Aug 28, 2021

Choose a reason for hiding this comment

ahmedaabouzied Sep 12, 2021

Choose a reason for hiding this comment

BenBE commented Oct 15, 2021

BenBE left a comment

Choose a reason for hiding this comment

BenBE commented Nov 28, 2021

ahmedaabouzied commented Nov 30, 2021

cgzones commented Dec 5, 2021

macmbello commented Mar 7, 2022

BenBE commented Jan 18, 2025

BenBE Jan 18, 2025

Choose a reason for hiding this comment

ahmedaabouzied commented Aug 16, 2021 •

edited

Loading

ahmedaabouzied commented Aug 24, 2021 •

edited

Loading