Skip to content
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

library: Add Check Button demo #337

Merged
merged 7 commits into from
Jul 13, 2023
Merged

library: Add Check Button demo #337

merged 7 commits into from
Jul 13, 2023

Conversation

AkshayWarrier
Copy link
Contributor

@AkshayWarrier AkshayWarrier commented Jun 14, 2023

Closes #336
Right now the inconsistent checkbox is not functioning properly, after it has completed one cycle of inconsistent -> on -> off -> inconsistent, the next time the check button turns on "Mark as Done: On" is logged twice because of checkbutton3.active = true under case 0, the button triggers ::toggled again.

A review would be appreciated.

@andyholmes
Copy link
Contributor

andyholmes commented Jun 15, 2023

So I think what you'll have to do is store a bit of state in an object/dictionary. Usually you'd have GSettings for this stuff somewhere, or state in your application.

These aren't used very often anymore, but they usually happen in a situation like this:

  • Inconsistent (not possible in markdown, I think)
    • Active
    • Inactive

It's not really important how you store the state for the demo, but something like this could work:

const state = {
    'checkbox': 0.5,
    'checkbox_child1': 1.0,
    'checkbox_child2': 0.0,
};

Three times: return the box to its mixed state, restoring each selected object’s original value for that setting.

This is the only tricky part about this, because you have to return the checkboxes to their original state. So my suggestion would be use the actual checkbox active states in your switch statement, only using checkbox_child1 and checkbox_child2 when restoring the original values.

In other words, if the parent is toggled you should only update state.checkbox and leave state.checkbox_child1 and state.checkbox_child1, while updating the child widgets. However, if the children are toggled, you should then update everything.

Does that makes sense?

@sonnyp
Copy link
Contributor

sonnyp commented Jun 25, 2023

@andyholmes could you help @AkshayWarrier with this one?

@andyholmes
Copy link
Contributor

Here's what I came up with, but check it out because I may have missed a corner case:

import GObject from "gi://GObject";

const checkbox_all = workbench.builder.get_object("checkbox_all");
const checkbox2 = workbench.builder.get_object("checkbox2");
const checkbox3 = workbench.builder.get_object("checkbox3");

// Define some mock settings and perform setup
const settings = {
  checkbox_all: false,
  checkbox2: false,
  checkbox3: true,
};

checkbox_all.active = settings.checkbox_all;
checkbox_all.inconsistent = settings.checkbox2 !== settings.checkbox3;
checkbox2.active = settings.checkbox2;
checkbox3.active = settings.checkbox3;

/* GNOME HIG for inconsistent checkbox states
 *
 * Behaviour pattern starts from an inconsistent state
 */
function toggleAllCallback(checkbutton) {
  console.log(`toggleAllCallback(${checkbutton.label})`);

  GObject.signal_handlers_block_by_func(checkbox2, toggleOneCallback);
  GObject.signal_handlers_block_by_func(checkbox3, toggleOneCallback);

  // Once: check the box and apply the setting to all the
  // selected objects.
  if (!settings.checkbox_all && checkbox_all.inconsistent) {
    checkbox_all.active = true;
    checkbox_all.inconsistent = false;

    checkbox2.active = true;
    checkbox3.active = true;

    // Twice: uncheck the box and unset the setting to all
    // the selected objects.
  } else if (settings.checkbox_all && !checkbox_all.inconsistent) {
    checkbox_all.active = false;
    checkbox_all.inconsistent = false;

    checkbox2.active = false;
    checkbox3.active = false;

    // Three times: return the box to its mixed state, restoring
    // each selected object’s original value for that setting.
  } else {
    checkbox_all.active = settings.checkbox2 && settings.checkbox2;
    checkbox_all.inconsistent = settings.checkbox2 !== settings.checkbox3;

    checkbox2.active = settings.checkbox2;
    checkbox3.active = settings.checkbox3;
  }

  // Update the mock setting
  settings.checkbox_all = checkbox_all.active;

  GObject.signal_handlers_unblock_by_func(checkbox2, toggleOneCallback);
  GObject.signal_handlers_unblock_by_func(checkbox3, toggleOneCallback);
}

function toggleOneCallback(checkbutton) {
  console.log(`toggleOneCallback(${checkbutton.label})`);

  GObject.signal_handlers_block_by_func(checkbox_all, toggleAllCallback);

  if (checkbox2.active && checkbox3.active) {
    settings.checkbox_all = true;
    checkbox_all.active = true;
    checkbox_all.inconsistent = false;
  } else if (checkbox2.active || checkbox3.active) {
    settings.checkbox_all = false;
    checkbox_all.active = false;
    checkbox_all.inconsistent = true;
  } else {
    settings.checkbox_all = false;
    checkbox_all.active = false;
    checkbox_all.inconsistent = false;
  }

  // Update the mock settings
  settings.checkbox2 = checkbox2.active;
  settings.checkbox3 = checkbox3.active;

  GObject.signal_handlers_unblock_by_func(checkbox_all, toggleAllCallback);
}

checkbox_all.connect("toggled", toggleAllCallback);
checkbox2.connect("toggled", toggleOneCallback);
checkbox3.connect("toggled", toggleOneCallback);

Copy link
Contributor

@andyholmes andyholmes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks almost good to go, now that we're waiting on the "inconsistent" state. Let's get this one cleaned up for the standard use and merge what we have.

src/Library/demos/Check Button/main.js Outdated Show resolved Hide resolved
src/Library/demos/Check Button/main.blp Outdated Show resolved Hide resolved
src/Library/demos/Check Button/main.blp Outdated Show resolved Hide resolved
src/Library/demos/Check Button/main.js Outdated Show resolved Hide resolved
@AkshayWarrier
Copy link
Contributor Author

Maybe a logged message that lists all selected boxes.

I decided to individually log the states of the checkboxes to show their independence from each other, also it makes the demo more copiable :)

@AkshayWarrier AkshayWarrier marked this pull request as ready for review July 12, 2023 22:33
Copy link
Contributor

@andyholmes andyholmes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks pretty much good to go.

A few formatting cleanups, also the last LinkButton I couldn't add a suggestion for, but the properties need to be indented.

After that looks good.

src/Library/demos/Check Button/main.blp Outdated Show resolved Hide resolved
src/Library/demos/Check Button/main.blp Outdated Show resolved Hide resolved
src/Library/demos/Check Button/main.blp Show resolved Hide resolved
src/Library/demos/Check Button/main.blp Show resolved Hide resolved
src/Library/demos/Check Button/main.blp Show resolved Hide resolved
src/Library/demos/Check Button/main.blp Outdated Show resolved Hide resolved
AkshayWarrier and others added 3 commits July 13, 2023 11:44
Co-authored-by: Andy Holmes <1265208+andyholmes@users.noreply.github.com>
Copy link
Contributor

@andyholmes andyholmes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well done, very clean and understandable. 👍

@andyholmes andyholmes merged commit 4011f83 into main Jul 13, 2023
@sonnyp sonnyp deleted the akshaywarrier/checkbutton branch July 13, 2023 20:33
sonnyp pushed a commit to SoNiC-HeRE/Workbench that referenced this pull request Aug 13, 2023
* library: Add Check Button demo

* Check Button: Attempt at making inconsistent checkbox work

* Check Button: Removed inconsistent buttons because they were inconsistent :)

* Apply suggestions from code review

Co-authored-by: Andy Holmes <1265208+andyholmes@users.noreply.github.com>

* Minor change

* Another minor change

* Last one

---------

Co-authored-by: Andy Holmes <1265208+andyholmes@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add CheckButton demo
3 participants