-
Notifications
You must be signed in to change notification settings - Fork 7
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(storybook): add functionality #1147
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @bar-tay ,
from looking at storybook, you managed to complete all ACs, but you also broke some existing behaviour.
For me it seems like the width of the text area is now only controlled by the parent. The old option to define the width via the cols prop is gone. This should not have happened. We want both options, defining the width by the parent or defining the width via the cols. For this we probably need another property, similar to how the buttonDisplay prop in the Button Text component.
If you are decide for adding a new property, please place it under the "Appearance" category and also add a Description. While at it, please also add a short description to the "rows" property, which explains that we have a min height.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @bar-tay ,
I see a few things that we need to look into:
-
If set to inline block, it seem like only the input field gets set to inline-block and the label container as well as the caption & counter container are still using the whole available width. This leads to the following unwanted behaviour:
Instead the behaviour should be the same as on the current develop. -
I see in the code, that you are using tokens for the MinHeight. I am not sure if they are the correct ones though, so please double check. In Figma it says to me the token for the LG viewport is called "Forms-TextArea-InputField-MinHeight-LG", in the code it looks to me you are using a token called "InputField.MinHeight.LG". Could you verify that you are using the correct token?
-
Only a small thing: Could you rename the "textareaDisplay" prop to "textAreaDisplay" and also place it below the "resize" and above the "cols" prop? Also please change the control form a select to a radio with both options.
-
Only a small thing: The "buttonDisplay" prop in the Button Text component seems to have been broken on your branch. There is no control and no description for the property.
-
Please have a look at the failing test and fix it as well.
For me, the state in develop also doesnt work. In my current state, I changed it so the label and the counter align with the textarea box when the size is changed. I also integrated that the appendix doesn't break when the label is super long. Maybe you have some input for that on further behaviour in the current state.
Yes, the correct values are used. I am destructuring the InputField.MinHeight... form the sem.TextArea in line 100 of the code. It is basically the same as when I would write TextArea.InputField.MinHeight... in a shorter form.
Done
Done
Done |
I already merged, as it looked good on my side as well and there was another approval. So this is my approval as well :-) |
* fix(storybook): add functionality * fix(storybook): textarea size limitations * fix(storybook): add row describtion * fix(storybook): review changes --------- Co-authored-by: Thorben Hartmann <122102805+thrbnhrtmnn@users.noreply.github.com>
closes #1113