feat(ui5-ai-button): implement split button functionality #10242

NHristov-sap · 2024-11-22T15:10:12Z

There is a request for redesign/refactoring of AI Button ui5-ai-button in order to use ui5-split-button instead of ui5-button which will allow app developers to define default action on pressing of AI button and opening an additional menu in some cases.

There is new property splitMode introduced in ui5-button-state component that activates split button mode for specific state.

…utton

hinzzx

We could also add a playground sample and a test maybe.
For example we can test that it changes successfully to split-button mode, when clicking Generate

packages/main/src/SplitButton.ts

packages/ai/src/Button.ts

Stoev

Hi, made some suggestions. Was not sure whether some of the instructions are intended for the developer, or for the end user. For example in lines 35 to 37 my assumption is that instruction is intended for developer, although some parts can be interpreted as intended for end user.

packages/ai/src/Button.ts

packages/ai/src/ButtonState.ts

packages/ai/src/Button.ts

packages/main/src/SplitButton.ts

Stoev

Please consider editing the capitalization for button names, as suggested in the comments.

packages/website/docs/_components_pages/ai/Button/Button.mdx

packages/ai/src/Button.ts

Stoev · 2024-12-19T11:19:59Z

packages/ai/src/Button.ts

- * Each state have a name that identifies it and can have text, icon and end icon defined (in any combination) depending on the state purpose.
+ * For the `ui5-ai-button` user interface, you can define one or more button states by placing `ui5-ai-button-state` components in their default slot.
+ * Each state has a name for identification and can include text, an icon, and an end icon, as needed for its purpose.
+ * You can define a split mode for the `ui5-ai-button`, which will results in displaying an arrow button for additional actions.


Please change:

You can define a split mode for the ui5-ai-button, which will results in displaying an arrow button for additional actions.

To:

You can define a split mode for the ui5-ai-button, which will result in displaying an arrow button for additional actions.

Stoev · 2024-12-19T11:56:53Z

packages/ai/src/Button.ts

- * It enables users to trigger actions by clicking or tapping the `ui5-ai-button`, or by pressing
- * certain keyboard keys, such as Enter.
+ * The `ui5-ai-button` component serves as a button for AI-related scenarios. Users can trigger actions by clicking or tapping the `ui5-ai-button`
+ * or by pressing keyboard keys like [Enter] or [Space].


Please consider changing:

Users can trigger actions by clicking or tapping the ui5-ai-button

To:

Users can trigger actions by choosing the ui5-ai-button

Stoev · 2024-12-19T12:00:31Z

packages/ai/src/Button.ts

+ * The `ui5-ai-button` can be activated by clicking or tapping it. You can change the button state in the click event handler. When the button is
+ * in split mode, you can activate the default button action by clicking or tapping it, or by pressing keyboard keys like [Enter] or [Space].
+ * You can activate the arrow button by clicking or tapping it, or by pressing keyboard keys like [Arrow Up], [Arrow Down], or [F4].
+ * To display additional actions, you can attach a menu to the arrow button.


Please consider changing for accessibility purpose:

The ui5-ai-button can be activated by choosing it. You can change the button state in the click event handler. When the button is in split mode, you can activate the default button action by choosing it or by pressing keyboard keys like [Enter] or [Space]. You can activate the arrow button by choosing it or by pressing keyboard keys like [Arrow Up], [Arrow Down], or [F4]. To display additional actions, you can attach a menu to the arrow button.

Stoev · 2024-12-19T12:05:22Z

packages/ai/src/Button.ts

+ * Fired when the component is in split mode and after the arrow button
+ * is activated either by clicking or tapping it or by using the [Arrow Up] / [Arrow Down],
+ * [Alt] + [Arrow Up]/ [Arrow Down], or [F4] keyboard keys.
+ * @public


Please consider changing:

аctivated either by clicking or tapping it

to:

аctivated by choosing it

vladitasev

Important: This is just a review of the public APIs

I see 2 issues:

The names of the new APIs are inconsistent

ai-button: activeArrowButton + arrow-click event
ai-button-state: splitMode

There is no way for the user to understand that these 3 are related and affect the same UI element (the arrow to the right)

Here are a couple of possibilities:

ai-button: arrowButtonActive + arrow-button-click event
ai-button-state: showArrowButton

or

ai-button: arrowActive + arrow-click event
ai-button-state: showArrow

I believe "active" is not the correct term for the pressed state of the arrow. Active means that the user can interact with it. Probably should be "pressed" - this is how it is in ui5-toggle-button

Therefore, my final suggestion would be:

ai-button: arrowButtonPressed + arrow-button-click event
ai-button-state: showArrowButton

This way it should be easier for app developers to connect the dots - they can set showArrowButton on the states, then they can set arrowButtonPressed on the ai button itself, and when the user clicks it the event that is fired is arrowButtonClick

vladitasev · 2024-12-20T08:02:03Z

packages/ai/src/Button.ts

-	@property({ type: Boolean })
-	fadeOut = false;
+	@property({ type: Boolean, noAttribute: true })
+	activeArrowButton = false;


Must be arrowButtonActive (boolean property names end with an adjective)

vladitasev · 2024-12-20T08:05:38Z

packages/ai/src/ButtonState.ts

+	 * @public
+	 */
+	@property({ type: Boolean })
+	splitMode = false;


I believe this should be called showArrowButton because splitMode means nothing for people who haven't seen the specs/figmas and is more like designer slang IMO

Stoev · 2024-12-20T08:33:48Z

[heart] Stoev, Dimitar reacted to your message:

________________________________ From: Vladislav Tasev ***@***.***> Sent: Friday, December 20, 2024 8:31:44 AM To: SAP/ui5-webcomponents ***@***.***> Cc: Stoev, Dimitar ***@***.***>; Comment ***@***.***> Subject: Re: [SAP/ui5-webcomponents] feat(ui5-ai-button): implement split button functionality (PR #10242) @vladitasev commented on this pull request. Important: This is just a review of the public APIs I see 2 issues: * The names of the new APIs are inconsistent ai-button: activeArrowButton + arrow-click event ai-button-state: splitMode There is no way for the user to understand that these 3 are related and affect the same UI element (the arrow to the right) Here are a couple of possibilities: ai-button: arrowButtonActive + arrow-button-click event ai-button-state: showArrowButton or ai-button: arrowActive + arrow-click event ai-button-state: showArrow 1. I believe "active" is not the correct term for the pressed state of the arrow. Active means that the user can interact with it. Probably should be "pressed" - this is how it is in ui5-toggle-button Therefore, my final suggestion would be: ai-button: arrowButtonPressed + arrow-button-click event ai-button-state: showArrowButton This way it should be easier for app developers to connect the dots - they can set showArrowButton on the states, then they can set arrowButtonPressed on the ai button itself, and when the user clicks it the event that is fired is arrowButtonClick

________________________________ In packages/ai/src/Button.ts<#10242 (comment)>:

*/

- @Property({ type: Boolean }) - fadeOut = false; + @Property({ type: Boolean, noAttribute: true }) + activeArrowButton = false; Must be arrowButtonActive (boolean property names end with an adjective)

________________________________ In packages/ai/src/ButtonState.ts<#10242 (comment)>:

@@ -67,6 +67,15 @@ class ButtonState extends UI5Element {

*/ @Property() endIcon?: string; + + /** + * Defines if the component is in split button mode. + * @default false + * @SInCE 2.6.0 + * @public + */ + @Property({ type: Boolean }) + splitMode = false; I believe this should be called showArrowButton because splitMode means nothing for people who haven't seen the specs/figmas and is more like designer slang IMO — Reply to this email directly, view it on GitHub<#10242 (review)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ADMB2UABOFVC4GAL7V2WIO32GPIXBAVCNFSM6AAAAABSJRZP5WVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDKMJWHE4DENBXHA>. You are receiving this because you commented.Message ID: ***@***.***>

ui5-webcomponents-bot · 2024-12-26T08:13:40Z

🎉 This PR is included in version v2.6.0-rc.3 🎉

The release is available on v2.6.0-rc.3

Your semantic-release bot 📦🚀

NHristov-sap added 4 commits November 20, 2024 10:10

feat(ui5-ai-button): implement split button functionality

dbe4117

feat(ui5-ai-button): fix animations

e7e6a50

feat(ui5-ai-button): fix accessibility features

7fab7ee

feat(ui5-ai-button): add new property for active state of the arrow b…

91bf41c

…utton

NHristov-sap self-assigned this Nov 26, 2024

NHristov-sap requested review from hinzzx and tsanislavgatev November 26, 2024 09:09

NHristov-sap added 5 commits November 26, 2024 16:31

feat(ui5-ai-button): fix arrow button active state in samples

702fe4a

feat(ui5-ai_button): fix lint errors

d0e487a

feat(ui5-ai_button): fix lint errors

2942ea5

feat(ui5-ai_button): fix lint errors

10502bd

feat(ui5-ai-button): fiox documentation and samples

4bd6ca3

hinzzx requested changes Dec 6, 2024

View reviewed changes

packages/main/src/SplitButton.ts Outdated Show resolved Hide resolved

packages/ai/src/Button.ts Outdated Show resolved Hide resolved

packages/ai/src/Button.ts Show resolved Hide resolved

packages/ai/src/Button.ts Outdated Show resolved Hide resolved

Stoev reviewed Dec 10, 2024

View reviewed changes

NHristov-sap added 2 commits December 11, 2024 10:00

feat(ui5-ai-button): fix comments

de88c1e

feat(ui5-ai-button): update templates to jsx

6fbfe56

Stoev reviewed Dec 16, 2024

View reviewed changes

packages/website/docs/_components_pages/ai/Button/Button.mdx Outdated Show resolved Hide resolved

NHristov-sap added 4 commits December 17, 2024 14:25

feat(ui5-ai-button): refactor animations

8b72f05

feat(ui5-ai-button): fix comments

4dac83e

feat(ui5-ai-button): fix lint errors

068bb03

Merge branch 'main' into BL_aibutton_split

79915ea

NHristov-sap requested review from Stoev and hinzzx December 17, 2024 13:52

hinzzx requested changes Dec 18, 2024

View reviewed changes

packages/ai/src/Button.ts Show resolved Hide resolved

packages/ai/src/Button.ts Show resolved Hide resolved

packages/ai/src/Button.ts Show resolved Hide resolved

packages/ai/src/Button.ts Show resolved Hide resolved

packages/ai/src/Button.ts Show resolved Hide resolved

feat(ui5-ai_button): fix documantation

42eb9c8

Stoev reviewed Dec 19, 2024

View reviewed changes

hinzzx approved these changes Dec 19, 2024

View reviewed changes

NHristov-sap requested review from ivtasev and vladitasev and removed request for ivtasev December 19, 2024 12:47

NHristov-sap requested review from nnaydenow and pskelin December 19, 2024 12:48

NHristov-sap mentioned this pull request Dec 19, 2024

PoC: AI Button Animation #10424

Closed

vladitasev reviewed Dec 20, 2024

View reviewed changes

vladitasev approved these changes Dec 20, 2024

View reviewed changes

NHristov-sap merged commit 55c9e4d into main Dec 20, 2024
10 checks passed

vladitasev deleted the BL_aibutton_split branch December 20, 2024 09:12

ui5-webcomponents-bot added the released label Dec 26, 2024

feat(ui5-ai-button): implement split button functionality #10242

feat(ui5-ai-button): implement split button functionality #10242

Uh oh!

Conversation

NHristov-sap commented Nov 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hinzzx left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Stoev left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Stoev left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Stoev Dec 19, 2024

Choose a reason for hiding this comment

Uh oh!

Stoev Dec 19, 2024

Choose a reason for hiding this comment

Uh oh!

Stoev Dec 19, 2024

Choose a reason for hiding this comment

Uh oh!

Stoev Dec 19, 2024

Choose a reason for hiding this comment

Uh oh!

vladitasev left a comment

Choose a reason for hiding this comment

Uh oh!

vladitasev Dec 20, 2024

Choose a reason for hiding this comment

Uh oh!

vladitasev Dec 20, 2024

Choose a reason for hiding this comment

Uh oh!

Stoev commented Dec 20, 2024 via email

Uh oh!

Uh oh!

ui5-webcomponents-bot commented Dec 26, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

NHristov-sap commented Nov 22, 2024 •

edited

Loading