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

feat: form and group dedicated styles #258

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

nervo
Copy link
Contributor

@nervo nervo commented May 28, 2024

This pr follows this one: #257

Instead of having only one style for form and groups in the theme, we introduce two new structs FormStyles and GroupStyles on the same model as FieldStyles. Both of them having a Base style applied to the View.

Please note that ErrorMessage have also been moved from FieldStyles to GroupStyles, as its only used in group view, and in a way that suggests there was a kind of mistake :)

Note also that since both Form and Group theme struct fields are unused today, we may consider that it's not a breaking change.

@nervo nervo requested a review from maaslalani as a code owner May 28, 2024 08:51
@maaslalani
Copy link
Contributor

@nervo, I think we shouldn't break the Theme API unless we have a super compelling reason to. I propose we simply start using the Form and Group styles in this PR instead of changing them to FormStyles and GroupStyles

@maaslalani
Copy link
Contributor

If we need Error / ErrorMessage style we can add that as a separate field on the theme.

@nervo nervo force-pushed the form-and-group-dedicated-styles branch from 909d9d9 to 01b7308 Compare June 21, 2024 12:26
@nervo
Copy link
Contributor Author

nervo commented Jun 21, 2024

@maaslalani using Form and Group styles is exactly (and only) the scope of this pr: #257 (which i just rebased)

If you're ok with it, please, consider validating and merging it :)

After that we can discuss the case of the error style on this thread.

@nervo nervo force-pushed the form-and-group-dedicated-styles branch from 01b7308 to 5ca34a5 Compare June 21, 2024 18:43
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