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

Enhance form and widget button label functionality #28

Merged
merged 5 commits into from
Jan 5, 2024

Conversation

yangbobo2021
Copy link
Contributor

@yangbobo2021 yangbobo2021 commented Jan 5, 2024

This pull request introduces two key features to enhance the user interface components:

  • A default_selected parameter has been added to the Radio widget, allowing a default option to be specified and visually marked when the widget is rendered.
  • Customizable button labels have been implemented, which enable different submit and cancel button names for the Form and TextEditor classes. Additionally, the chatmark header in the Widget class has been updated to reflect these custom labels. This change also affects the initialization of Checkbox and Radio classes.

These updates aim to provide a more flexible and user-friendly experience with our UI components.

As there was no specific issue linked to these changes, there is no issue to close with this pull request.

@yangbobo2021 yangbobo2021 requested a review from kagami-l January 5, 2024 00:46
Copy link
Collaborator

@kagami-l kagami-l left a comment

Choose a reason for hiding this comment

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

  1. radio的设计也更新了,需要改
  2. button不需要这两个按钮,加了的话得看前端实现会不会忽略
  3. submit/cancel两个值最好不是作为render函数的参数,而是__init__参数,并且可作为widget的property

参考设计文档变更:devchat-ai/devchat-website#4

@yangbobo2021 yangbobo2021 force-pushed the enhance/form-button-labels branch from 367fbe9 to 9c3ebd0 Compare January 5, 2024 02:47
@yangbobo2021 yangbobo2021 requested a review from kagami-l January 5, 2024 03:12
@@ -9,10 +9,12 @@ class Widget(ABC):
Abstract base class for widgets
"""

def __init__(self):
def __init__(self, submit: Optional[str] = None, cancel: str = "Cancel"):
Copy link
Collaborator

Choose a reason for hiding this comment

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

cancel也应该是 Optional的吧

@@ -249,12 +265,12 @@ def __init__(
# if default_selected is not None:
# assert 0 <= default_selected < len(options)
Copy link
Collaborator

Choose a reason for hiding this comment

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

加了default_selected后,这段注释也恢复一下,检查下标是否越界

Comment on lines 301 to 302
if self._selection is not None and self._selection == idx:
lines.append(f"> X ({key}) {option}")
Copy link
Collaborator

Choose a reason for hiding this comment

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

可以只判断 if self._selection == idx:,另外文档里x是小写,不确定前端解析到时是不是case-sensitive,这边还是按文档来吧

- Form and TextEditor classes now accept submit_button_name and cancel_button_name
- Widget's chatmark header updated with customizable submit and cancel labels
- Adjusted initialization of Checkbox and Radio classes with button label parameters
- Introduced the default_selected parameter in the Radio widget
- Updated Radio widget to mark default selected option visually
- Removed TODO comment for the default_selected implementation
- Add trailing commas to button name parameters in widgets
- Reformat constructor definitions for consistency
- Clean up unnecessary whitespace in widget render logic
- Make submit and cancel button labels optional in form and widgets
- Refactor chatmark header construction for conditional button labels
- Implement assertion for Radio widget's default_selected option
@kagami-l kagami-l force-pushed the enhance/form-button-labels branch from bb48e77 to 05d8e6d Compare January 5, 2024 04:01
@kagami-l kagami-l merged commit 13fe016 into main Jan 5, 2024
1 check passed
@kagami-l kagami-l deleted the enhance/form-button-labels branch January 11, 2024 07:37
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.

2 participants