Fix FwbInput label/input connection #405
Conversation
…or the input element if a label prop has been passed and is not an empty string. preserves any attrs passed to fwb-input. lowercased, trimmed and replaced spaces on the label to create an id. updated the docs to include the autocomplete prop. moved the InputProps interface to the FwbInput/types file
WalkthroughThis change refactors the Changes
Sequence Diagram(s)sequenceDiagram participant ParentComponent participant FwbInput participant useInputAttributes ParentComponent->>FwbInput: Passes props and attributes FwbInput->>useInputAttributes: Calls with props useInputAttributes-->>FwbInput: Returns inputId, inputAttributes FwbInput->>FwbInput: Binds inputAttributes to <input>, inputId to <label> Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File ( |
✅ Deploy Preview for sensational-seahorse-8635f8 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/components/FwbInput/composables/useInputAttributes.ts (1)
9-18: Consider making inputId reactive for consistency.The
inputAttributescomputed works correctly, butinputIdshould also be computed to ensure reactivity when the label prop changes.- const inputId = props?.label && props.label !== '' ? props.label.toLowerCase().trim().replace(/ /g, '-') : '' + const inputId = computed(() => props?.label && props.label !== '' ? props.label.toLowerCase().trim().replace(/ /g, '-') : '') const inputAttributes = computed(() => { - if (inputId !== '') { + if (inputId.value !== '') { return { ...attrs, - id: inputId, + id: inputId.value, } } else { return attrs } })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
docs/components/input.md(1 hunks)src/components/FwbInput/FwbInput.vue(4 hunks)src/components/FwbInput/composables/useInputAttributes.ts(1 hunks)src/components/FwbInput/types.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: Sqrcz PR: themesberg/flowbite-vue#338 File: docs/components/dropdown.md:243-261 Timestamp: 2025-01-11T13:26:22.855Z Learning: The `disabled` prop in the FwbDropdown component is a boolean prop that defaults to `false` and controls whether the dropdown trigger button is disabled. 📚 Learning: the `disabled` prop in the fwbdropdown component is a boolean prop that defaults to `false` and cont...
Learnt from: Sqrcz PR: themesberg/flowbite-vue#338 File: docs/components/dropdown.md:243-261 Timestamp: 2025-01-11T13:26:22.855Z Learning: The `disabled` prop in the FwbDropdown component is a boolean prop that defaults to `false` and controls whether the dropdown trigger button is disabled. Applied to files:
docs/components/input.md
🧬 Code Graph Analysis (1)
src/components/FwbInput/composables/useInputAttributes.ts (1)
src/components/FwbInput/types.ts (1)
InputProps(8-20)
🔇 Additional comments (5)
src/components/FwbInput/types.ts (1)
8-20: Well-designed centralized interface.The
InputPropsinterface effectively centralizes all input component properties with appropriate optional typing. The use of existing types likeCommonAutoFill,InputSize,InputType, andValidationStatusmaintains consistency across the codebase.docs/components/input.md (1)
257-263: Documentation accurately reflects the new autocomplete prop.The addition of the
autocompleteprop documentation is consistent with the interface definition and follows the established documentation format.src/components/FwbInput/FwbInput.vue (3)
5-5: Excellent accessibility improvement with label association.The
:for="inputId"binding properly connects the label to the input element, addressing the core accessibility issue mentioned in the PR objectives.
50-53: Clean refactor with proper composable integration.The integration of
useInputAttributescomposable and centralizedInputPropstype creates a more maintainable architecture while successfully addressing the label/input association issue.Also applies to: 84-84
16-22: No attribute duplication risk:autocompleteis a prop, not included ininputAttributes
SinceinheritAttrs: falseandautocompleteis declared indefineProps,useAttrs()will strip out any prop‐named attributes. The computedinputAttributesonly contains non-prop attrs (plusid), so there’s no chance of forwardingautocompletetwice. Keeping the explicit:autocomplete="autocomplete"binding is both necessary and correct.
Likely an incorrect or invalid review comment.
| Hey @ChrisWSchultz 👋 Thank you for your input... it looks very good and the description is 👌 I'm on holidays until Monday... if I find a moment to properly go through it... I will... but probably I'll take care of it somewhere next week. |
| return props.label | ||
| .toLowerCase() | ||
| .trim() | ||
| .replace(/[^\w\s-]/g, '') // Remove special characters | ||
| .replace(/\s+/g, '-') // Replace spaces with hyphens | ||
| .replace(/-+/g, '-') // Collapse multiple hyphens | ||
| }) |
There was a problem hiding this comment.
Consider enhancing the slug generation logic to handle edge cases with special or accented characters in future iterations. Currently, strings like ąśćźż result in empty outputs, which may leave elements without valid id attributes. Using a transliteration solution (such as any-ascii), or alternatively generating a random string fallback when the result is empty, could improve robustness and accessibility.
The
FwbInputinput element is missing aid/forconnection with the label element.There are divs wrapping the input element, so the label can't be used to wrap.
As a side-effect this also fixes the
FwbAutocompletelabel as well.Please let me know if/what changes I need to make. Thank you!
src/components/FwbInput/FwbInput.vue:foron the label elementv-bindto useinputAttributesuseInputAttributesInputPropsinterface the the types.ts filesrc/components/FwbInput/composables/useInputAttributes.ts:idand:foron the input and label, but if no label is given there is a null/undefined id left on the input:forattributesrc/components/FwbInput/types.tsInputPropsinterface to be able to import it inuseInputAttributesdocs/components/input.mdautocompleteprop to the input component docsSummary by CodeRabbit
Summary by CodeRabbit
New Features
autocompleteoption to the input component, allowing users to control browser autocomplete behavior.Documentation
autocompleteoption and clarified prop types for consistency.