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

Button default name #1243

Merged
merged 10 commits into from
May 13, 2022
Merged

Button default name #1243

merged 10 commits into from
May 13, 2022

Conversation

omerXfaruq
Copy link
Contributor

@omerXfaruq omerXfaruq commented May 13, 2022

Description

Add a default button name since it was not looking good in default.

Before:
image

Now:
image

- fix&update gr.component
- create a demo introducing shortcuts within Blocks
- fix formatting
- tweaks
- fix tests
- tweaks
- fix tests
- tweaks
- fix tests
- add a default name in Button
- add a default name in Button
@abidlabs
Copy link
Member

Looks good, I think this is a good change. The other alternative would be just to make a Button's value a required parameter, but this works as well. Let's get feedback from @gary149 or @pngwn as well.

@pngwn
Copy link
Member

pngwn commented May 13, 2022

I think "Run" is a reasonable default, although the sole puprose of this button is to render some text, I don't think asking users to pass in a value is unreasonable. I'd be happy with either the default or making it required.

@gary149
Copy link
Contributor

gary149 commented May 13, 2022

Looks good, I think this is a good change. The other alternative would be just to make a Button's value a required parameter, but this works as well. Let's get feedback from @gary149 or @pngwn as well.

I think it makes more sense as a required param (so buttons always get the right wording), but no super strong opinion on it.

@pngwn
Copy link
Member

pngwn commented May 13, 2022

Lets make it required then, seems reasonable.

@omerXfaruq
Copy link
Contributor Author

omerXfaruq commented May 13, 2022

If there is no strong opinion on this I would suggest we continue with default values.
Since we are supporting component shortcuts in the Blocks as well, component shortcuts are initializing components with default values. Having default values for components would support shortcuts, and enable very easy peasy usage. Run is very generic and works well.

Check demo

@pngwn
Copy link
Member

pngwn commented May 13, 2022

The string shortcut is a good point. Lets merge this.

@omerXfaruq omerXfaruq merged commit 5df69c1 into main May 13, 2022
@omerXfaruq omerXfaruq deleted the button-default-name branch May 13, 2022 17:18
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.

4 participants