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

fix: Let wave app programmatically change value and choices for combobox #1538

Merged
merged 15 commits into from
Aug 8, 2022

Conversation

aalencar
Copy link
Contributor

closes #1501

Copy link
Collaborator

@mturoci mturoci left a comment

Choose a reason for hiding this comment

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

Thanks @aalencar, choices seem to be working fine, but the value needs more work:

Screen.Recording.2022-07-13.at.3.56.27.PM.mov

Repro code

# Form / Combobox
# Use comboboxes to allow users to either choose between available choices or indicate a choice by free-form editing.
# #form #combobox
# ---
from h2o_wave import main, app, Q, ui

combobox_choices = ['Cyan', 'Magenta', 'Yellow', 'Black']


@app('/demo')
async def serve(q: Q):
    if not q.client.initialized:
        q.page['example'] = ui.form_card(box='1 1 4 10', items=[
            ui.combobox(name='combobox', label='Enter or choose a color', placeholder='Color...', value='Blue', choices=combobox_choices),
            ui.button(name='btn', label='Change value'),
        ])
        q.client.initialized = True
    if q.args.btn:
        q.page['example'].items[0].combobox.value = 'New value'
    await q.page.save()

* also simplify "selected" to contain an array of strings instead of object
@aalencar aalencar requested a review from mturoci July 19, 2022 15:08
Copy link
Collaborator

@mturoci mturoci left a comment

Choose a reason for hiding this comment

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

Please make sure unit tests are passing.

@aalencar aalencar requested a review from mturoci July 20, 2022 09:18
Copy link
Collaborator

@mturoci mturoci left a comment

Choose a reason for hiding this comment

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

Regression - clicking away should not clear the selection:

Screen.Recording.2022-07-20.at.12.29.55.PM.mov

@aalencar It would be great if you could do some QA yourself before submitting a PR. The recommended steps are:

  • Implement the fix.
  • Think of possible edge cases.
  • Check if unit tests are passing.
  • If working on something that can affect more components globally, check visual regression tests.
  • Do some manual QA to check there are no regressions.
  • If there are regressions, fix them and include them in the unit tests if possible (for quicker identification in the future).
  • If you encounter some other present bugs (not caused by the newest fix) you can either fix them as part of the current task if they are fairly small or file an issue to raise awareness.

I believe following this list can save us all a lot of time. You can consider these as a bare minimum, feel free to expand the list with your own ideas and workflows as you see fit.

Copy link
Collaborator

@mturoci mturoci left a comment

Choose a reason for hiding this comment

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

Thanks @aalencar, looking good except a few small things.

py/examples/combobox.py Outdated Show resolved Hide resolved
ui/src/combobox.test.tsx Outdated Show resolved Hide resolved
ui/src/combobox.test.tsx Outdated Show resolved Hide resolved
ui/src/combobox.tsx Outdated Show resolved Hide resolved
ui/src/combobox.tsx Outdated Show resolved Hide resolved
ui/src/combobox.tsx Outdated Show resolved Hide resolved
ui/src/combobox.tsx Outdated Show resolved Hide resolved
ui/src/combobox.tsx Outdated Show resolved Hide resolved
@aalencar
Copy link
Contributor Author

aalencar commented Aug 2, 2022

Current behavior:

  • Wave App can update "value" and "choices" dynamically
  • If "value" is not included in "choices" then add it to the option list
  • If Wave App updates "choices" only and current selected value is not in "choices" then clear combobox (consistent with other option-based components)

Terminology

  • "value": "value" argument set on Wave App
  • "choices": an array that represents options set on Wave App
  • options list: "choices" + user typed options (combobox listbox)
  • selected value: current select value in the combobox

Only remaining issue happens when you:

  • Use multi-select Combobox
  • Open list of options
  • Type a new value and press Enter

The displayed value will be the first option in the list instead of the previously selected values + the last typed one until you click away.

Screen.Recording.2022-08-02.at.15.38.55.mov

This looks like a Fluent bug though.

Screen.Recording.2022-08-02.at.15.40.11.mov

I tried to cover all the edge cases with tests. Let me know if I missed any.

What I have been using for testing:

from h2o_wave import main, app, Q, ui

combobox_choices = ['A', 'B', 'C']

@app('/demo')
async def serve(q: Q):
    if not q.client.initialized:
        q.page['example'] = ui.form_card(box='1 1 4 10', items=[
            ui.combobox(name='combobox', label='Enter or choose a color', placeholder='Color...', value='A', choices=combobox_choices),
            ui.combobox(name='combobox2', label='Enter or choose a color', placeholder='Color...', values=['A'], choices=combobox_choices),
            ui.button(name='btn1', label='value -> A'),
            ui.button(name='btn2', label='value -> B'),
            ui.button(name='btn3', label='value -> C'),
            ui.button(name='btn4', label='choices -> [A, B]'),
            ui.button(name='btn5', label='value -> "A" -> [A, B, D]'),
            ui.button(name='btn6', label='value -> "B" -> [A, B, D]'),
            ui.button(name='btn7', label='value -> "C" -> [A, B]'),
        ])
        q.client.initialized = True
    if q.args.btn1:
        q.page['example'].items[0].combobox.value = 'A'
        q.page['example'].items[1].combobox.values = ['A']
    if q.args.btn2:
        q.page['example'].items[0].combobox.value = 'B'
        q.page['example'].items[1].combobox.values = ['B']
    if q.args.btn3:
        q.page['example'].items[0].combobox.value = 'C'
        q.page['example'].items[1].combobox.values = ['C']
    if q.args.btn4:
        q.page['example'].items[0].combobox.choices = ['A', 'B']
        q.page['example'].items[1].combobox.choices = ['A', 'B']
    if q.args.btn5:
        q.page['example'].items[0].combobox.value = 'A'
        q.page['example'].items[0].combobox.choices = ['A', 'B', 'D']
        q.page['example'].items[1].combobox.values = ['A']
        q.page['example'].items[1].combobox.choices = ['A', 'B', 'D']
    if q.args.btn6:
        q.page['example'].items[0].combobox.value = 'B'
        q.page['example'].items[0].combobox.choices = ['A', 'B', 'D']
        q.page['example'].items[1].combobox.values = ['B']
        q.page['example'].items[1].combobox.choices = ['A', 'B', 'D']
    if q.args.btn7:
        q.page['example'].items[0].combobox.value = 'C'
        q.page['example'].items[0].combobox.choices = ['A', 'B']
        q.page['example'].items[1].combobox.values = ['C']
        q.page['example'].items[1].combobox.choices = ['A', 'B']
    await q.page.save()

@aalencar aalencar requested a review from mturoci August 2, 2022 14:26
Copy link
Collaborator

@mturoci mturoci left a comment

Choose a reason for hiding this comment

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

Thanks @aalencar, just a few last small comments.

The displayed value will be the first option in the list instead of the previously selected values + the last typed one until you click away.

Good catch! Please file a bug report to Fluent repo so that we are aware when the fix is available.

ui/src/combobox.tsx Outdated Show resolved Hide resolved
ui/src/combobox.tsx Outdated Show resolved Hide resolved
ui/src/combobox.tsx Outdated Show resolved Hide resolved
@aalencar
Copy link
Contributor Author

aalencar commented Aug 3, 2022

@aalencar aalencar requested a review from mturoci August 3, 2022 15:57
@mturoci mturoci merged commit 5eb050e into master Aug 8, 2022
@mturoci mturoci deleted the fix/issue-1501 branch August 8, 2022 06:31
@aalencar aalencar mentioned this pull request Sep 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

combobox cannot be updated in event handler
2 participants