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(NcTextArea): implement new textarea component #4605

Merged
merged 1 commit into from Oct 23, 2023

Conversation

st3iny
Copy link
Contributor

@st3iny st3iny commented Oct 4, 2023

☑️ Resolves

🖼️ Screenshots

Screenshot 2023-10-11 at 11-30-03 Nextcloud Vue Style Guide

🚧 Tasks

None

🏁 Checklist

  • ⛑️ Tests are included or are not applicable
  • 📘 Component documentation has been extended, updated or is not applicable

@st3iny st3iny added enhancement New feature or request 2. developing Work in progress labels Oct 4, 2023
@st3iny st3iny self-assigned this Oct 4, 2023
@st3iny st3iny force-pushed the feat/textarea/implement-new-component branch from eedddeb to 5ff9853 Compare October 11, 2023 09:30
@st3iny st3iny added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Oct 11, 2023
@st3iny st3iny marked this pull request as ready for review October 11, 2023 09:33
Signed-off-by: Richard Steinmetz <richard@steinmetz.cloud>
@st3iny st3iny force-pushed the feat/textarea/implement-new-component branch from 5ff9853 to 4d09444 Compare October 11, 2023 09:34
@st3iny st3iny added this to the 8.0.0 milestone Oct 11, 2023
Copy link
Contributor

@ShGKme ShGKme left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At a glance, this component looks very similar to NcInputField with similar interface, states, helper/hint text, public methods, and label. Except a trailing button.

Could it be possible to add a new prop in NcInputField like multiline/textarea instead and use dynamic component?

Maybe making this component a wrapper around NcInputField like NcTextField if we need to assign more custom things.

@st3iny
Copy link
Contributor Author

st3iny commented Oct 11, 2023

At a glance, this component looks very similar to NcInputField with similar interface, states, helper/hint text, public methods, and label. Except a trailing button.

Could it be possible to add a new prop in NcInputField like multiline/textarea instead and use dynamic component?

Maybe making this component a wrapper around NcInputField like NcTextField if we need to assign more custom things.

That was my plan at first but doing so would require a lot of hacks. For example trailing buttons, and leading icons don't make sense here. They look weird as the text extends to multiple lines.

The success and error states are tied to leading icons in NcInputField which would need to be hacked away when using them for NcTextArea.

I went with a custom component to reduce complexity and narrow the scope of NcInputField to make future changes easier.

@susnux
Copy link
Contributor

susnux commented Oct 13, 2023

I agree with @st3iny, the NcInputField is a wrapper around the native input field and this one is a wrapper around the native textarea.
This way we do not need to deal with two different elements, reducing the complexity on the component.

Copy link
Contributor

@GretaD GretaD left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@st3iny st3iny merged commit 1c71b1e into master Oct 23, 2023
15 checks passed
@st3iny st3iny deleted the feat/textarea/implement-new-component branch October 23, 2023 07:08
@Pytal Pytal mentioned this pull request Oct 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants