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

fix(ui): re-introduce fields unit in Builder - WF-127 #694

Merged
merged 1 commit into from
Dec 16, 2024

Conversation

madeindjs
Copy link
Collaborator

Restore the field unit removed in #657

before after
image image

@madeindjs madeindjs self-assigned this Dec 16, 2024
@ramedina86
Copy link
Collaborator

I think we should do same font-size for name and type. Inspiration is TypeScript e.g. ev: WheelEvent.

@madeindjs
Copy link
Collaborator Author

I think we should do same font-size for name and type. Inspiration is TypeScript e.g. ev: WheelEvent.

Implemented

@ramedina86
Copy link
Collaborator

Is unit the proper name of a type I don't know about? 😀

@madeindjs
Copy link
Collaborator Author

Is unit the proper name of a type I don't know about? 😀

I found type a bit too generic.

Since the WdsFieldWrapper is a WDS component, I thought it could be better to have a more meaningful name like unit. For example, we could reuse for padding fields.

image

But I won't fight to keep unit, I can use type if you prefer 😁

@ramedina86
Copy link
Collaborator

Ah I see, nah unit is fine, not the most descriptive name but I feel the pain with type - everything has a type.

@madeindjs madeindjs marked this pull request as ready for review December 16, 2024 11:38
@ramedina86 ramedina86 merged commit 94f6293 into writer:dev Dec 16, 2024
16 checks passed
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