Skip to content

Comments

fix(NcTextField): do not pass all props to InputField BY stop passing $props#4665

Closed
ShGKme wants to merge 1 commit intomasterfrom
fix/text-field--do-not-pass-extra-attrs-1
Closed

fix(NcTextField): do not pass all props to InputField BY stop passing $props#4665
ShGKme wants to merge 1 commit intomasterfrom
fix/text-field--do-not-pass-extra-attrs-1

Conversation

@ShGKme
Copy link
Contributor

@ShGKme ShGKme commented Oct 18, 2023

☑️ Resolves

HTML Validation issue.

Currently NcTextField passes both $attrs and $props to the NcInputField.

<NcInputField v-bind="{ ...$attrs, ...$props }">

While passing $attrs makes sense, passing $props is valid only when both components have exactly the same props. But they don't. NcTextField has its own prop trailingButtonIcon and may have more props in the future.


Proposal: same as NcPopover

Currently, all NcInputField props are defined as NcTextField props. But actually they are not NcTextField props. They are not used but NcTextField, it is not aware of most of them at all. It is used only for styleguidist documentation generation.

The proposal is to make component a transparent wrapper by passing attributes directly, defining only necessary props.

Alternative solution:

While I personally prefer this solution, there is an alternative: filter out props on proxying.

See: #4666

🖼️ Screenshots

trailingbuttonicon is not a valid <input> attribute.

image

🏁 Checklist

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

@ShGKme ShGKme added bug Something isn't working accessibility Making sure we design for the widest range of people possible, including those who have disabilities feature: input-field Covering the InputField, TextField, ... labels Oct 18, 2023
@ShGKme ShGKme added this to the 8.0.0 milestone Oct 18, 2023
@ShGKme ShGKme self-assigned this Oct 18, 2023
@szaimen

This comment was marked as resolved.

@szaimen szaimen removed their request for review October 19, 2023 09:21
props: {
...NcInputField.props,
/**
* Any NcInputField props
Copy link
Contributor

@Pytal Pytal Oct 19, 2023

Choose a reason for hiding this comment

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

Suggested change
* Any NcInputField props
* Any [NcInputField](#/Components/NcFields?id=ncinputfield) props

Copy link
Contributor Author

@ShGKme ShGKme Oct 19, 2023

Choose a reason for hiding this comment

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

I didn't know that's possible =D

@ShGKme
Copy link
Contributor Author

ShGKme commented Oct 21, 2023

Closing in flavor of an alternative approach.

@ShGKme ShGKme closed this Oct 21, 2023
@susnux susnux deleted the fix/text-field--do-not-pass-extra-attrs-1 branch April 2, 2025 11:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

accessibility Making sure we design for the widest range of people possible, including those who have disabilities bug Something isn't working feature: input-field Covering the InputField, TextField, ...

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants