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

Allow use of ui-kit icons on secondary button of CustomFormModalPage #1813

Open
dogayuksel opened this issue Oct 21, 2020 · 2 comments
Open
Labels
💅 Type: Enhancement Improves existing code

Comments

@dogayuksel
Copy link
Contributor

dogayuksel commented Oct 21, 2020

Problem

We would like to standardize save/cancel behavior across Merchant Center. Forms used for editing existing items should have a 'revert changes' button as a secondary action to cancel the changes. Forms used for adding new items should continue to use 'cancel'.

Revert changes button should ideally have the 'revert' icon.

Screenshot 2020-10-21 at 14 29 24

See UI-Team meeting notes for details: https://wiki.commercetools.com/pages/viewpage.action?pageId=193308973

See priceless issue for more details on the dependent task: https://jira.commercetools.com/browse/PRC-1261

Suggested Solution

As secondary button of ui-kit already supports iconLeft property, we should modify CustomFormModalPage to optionally accept it and pass it down.

Alternative Solution

Use ui-kit's secondaryButton instead the secondary button exposed by the CustomFormModalPage. However it would be cleaner to use the icon feature through the interface of CustomFormModalPage component.

@anilkk
Copy link
Contributor

anilkk commented Oct 22, 2020

@dogayuksel We appreciate your suggestion to improve the UX of CustomFormModalPage. We are working on defining the review process and guidelines for the contributions. We have to wait until a well-defined review process in place before we involve our UX designers to review.

@emmenko emmenko added this to Needs research in SHIELD Board - UI components via automation Nov 11, 2020
@mmaltsev-ct
Copy link
Contributor

@FilPob could you revisit the issue please, and check whether the UX team wants to have such functionality as standard behavior.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💅 Type: Enhancement Improves existing code
Projects
No open projects
Development

No branches or pull requests

3 participants