feat: Label added for FileInput dropzone #304
Conversation
WalkthroughThe recent updates to the FwbFileInput component enhance its layout and user experience. By shifting to a vertical arrangement for the file input area, the design allows for better organization of elements. The addition of a conditional label further guides users, improving accessibility and usability. These changes reflect a commitment to creating a more intuitive interface for file uploads. Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
✅ Deploy Preview for sensational-seahorse-8635f8 ready!
To edit notification comments on pull requests, go to your Netlify site 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: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- src/components/FwbFileInput/FwbFileInput.vue (1 hunks)
Additional comments not posted (2)
src/components/FwbFileInput/FwbFileInput.vue (2)
18-18: LGTM! Verify the overall layout.The change from
flex items-center justify-centertoflex flex-col items-start justify-centershifts the layout from horizontal to vertical alignment. This can improve the organization of elements within the dropzone. Ensure that this change does not negatively impact the overall layout and user experience.
23-23: LGTM! Verify the conditional rendering.The addition of the
<span>element to conditionally render based on the presence of alabelprop improves the component's usability by providing a visual cue for users regarding the expected input. Ensure that the conditional rendering works correctly and does not cause any issues.
cogor
added
🔧 enhancement
There was a problem hiding this comment.
Please fix linter issues... 🙏
@zoltanszogyenyi would like to know your opinion on this one...
Flowbite's dropzone also has no Label...
The way it is proposed in this PR (outside of <label> tag) could be accepted for now as a temporary solution but it's not perfect in terms of UX/Accessibility. Is there a chance to update Original Flowbite component to allow additional labels?
| @change="handleChange" | ||
| @dragover="dragOverHandler" | ||
| @drop="dropFileHandler" | ||
| > | ||
| <span v-if="label !== ''" :class="labelClasses">{{ label }}</span> |
There was a problem hiding this comment.
please fix code style according to linter (npm run lint)
/src/components/FwbFileInput/FwbFileInput.vue
23:1 warning Expected indentation of 6 spaces but found 4 spaces vue/html-indent
23:31 warning ':class' should be on a new line vue/max-attributes-per-line
There was a problem hiding this comment.
I did npm run format. Lint was giving too many errors.
There was a problem hiding this comment.
Actionable comments posted: 4
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- src/components/FwbFileInput/FwbFileInput.vue (5 hunks)
Additional context used
GitHub Check: lint (18.x)
src/components/FwbFileInput/FwbFileInput.vue
[warning] 6-6:
Disallow self-closing on HTML void elements ()
[warning] 6-6:
':multiple' should be on a new line
[warning] 6-6:
'type' should be on a new line
[warning] 6-6:
':accept' should be on a new line
[warning] 6-6:
'@change' should be on a new line
[warning] 10-10:
'class' should be on a new line
[warning] 10-10:
'@change' should be on a new line
[warning] 10-10:
'@dragover' should be on a new line
[warning] 10-10:
'@drop' should be on a new line
[warning] 11-11:
':class' should be on a new line
[failure] 77-77:
Missing space before function parentheses
[failure] 80-80:
Missing space before function parentheses
Additional comments not posted (3)
src/components/FwbFileInput/FwbFileInput.vue (3)
14-14: LGTM!The SVG element is correctly formatted and enhances the visual representation of the dropzone.
48-49: LGTM!The additions to the
FileInputPropsinterface are correct and enhance the component's functionality.
119-119: LGTM!The usage of
useFileInputClassesis correct and enhances the component's styling.
| @drop="dropFileHandler" | ||
| > | ||
| <div v-else class="flex flex-col items-start justify-center" @change="handleChange" @dragover="dragOverHandler" @drop="dropFileHandler"> | ||
| <span v-if="label !== ''" :class="labelClasses">{{ label }}</span> |
There was a problem hiding this comment.
Format the span element attribute.
The attribute of the <span> element should be on a separate line for better readability and to conform to the project's style guide.
- <span v-if="label !== ''" :class="labelClasses">{{ label }}</span>
+ <span
+ v-if="label !== ''"
+ :class="labelClasses"
+ >
+ {{ label }}
+ </span>Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <span v-if="label !== ''" :class="labelClasses">{{ label }}</span> | |
| <span | |
| v-if="label !== ''" | |
| :class="labelClasses" | |
| > | |
| {{ label }} | |
| </span> |
Tools
GitHub Check: lint (18.x)
[warning] 11-11:
':class' should be on a new line
| @dragover="dragOverHandler" | ||
| @drop="dropFileHandler" | ||
| > | ||
| <div v-else class="flex flex-col items-start justify-center" @change="handleChange" @dragover="dragOverHandler" @drop="dropFileHandler"> |
There was a problem hiding this comment.
Format the div element attributes.
The attributes of the <div> element should be on separate lines for better readability and to conform to the project's style guide.
- <div v-else class="flex flex-col items-start justify-center" @change="handleChange" @dragover="dragOverHandler" @drop="dropFileHandler">
+ <div
+ v-else
+ class="flex flex-col items-start justify-center"
+ @change="handleChange"
+ @dragover="dragOverHandler"
+ @drop="dropFileHandler"
+ >Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <div v-else class="flex flex-col items-start justify-center" @change="handleChange" @dragover="dragOverHandler" @drop="dropFileHandler"> | |
| <div | |
| v-else | |
| class="flex flex-col items-start justify-center" | |
| @change="handleChange" | |
| @dragover="dragOverHandler" | |
| @drop="dropFileHandler" | |
| > |
| get() { | ||
| return props.modelValue | ||
| }, | ||
| set (val) { | ||
| set(val) { |
There was a problem hiding this comment.
Add missing spaces before function parentheses.
There should be a space before the function parentheses for consistency and readability.
- get() {
+ get () {
- set(val) {
+ set (val) {Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| get() { | |
| return props.modelValue | |
| }, | |
| set (val) { | |
| set(val) { | |
| get () { | |
| return props.modelValue | |
| }, | |
| set (val) { |
Tools
GitHub Check: lint (18.x)
[failure] 77-77:
Missing space before function parentheses
[failure] 80-80:
Missing space before function parentheses
| :accept="accept" | ||
| @change="handleChange" | ||
| > | ||
| <input :class="fileInpClasses" :multiple="multiple" type="file" :accept="accept" @change="handleChange" /> |
There was a problem hiding this comment.
Format the input element attributes.
The attributes of the <input> element should be on separate lines for better readability and to conform to the project's style guide.
- <input :class="fileInpClasses" :multiple="multiple" type="file" :accept="accept" @change="handleChange" />
+ <input
+ :class="fileInpClasses"
+ :multiple="multiple"
+ type="file"
+ :accept="accept"
+ @change="handleChange"
+ />Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <input :class="fileInpClasses" :multiple="multiple" type="file" :accept="accept" @change="handleChange" /> | |
| <input | |
| :class="fileInpClasses" | |
| :multiple="multiple" | |
| type="file" | |
| :accept="accept" | |
| @change="handleChange" | |
| /> |
Tools
GitHub Check: lint (18.x)
[warning] 6-6:
Disallow self-closing on HTML void elements ()
[warning] 6-6:
':multiple' should be on a new line
[warning] 6-6:
'type' should be on a new line
[warning] 6-6:
':accept' should be on a new line
[warning] 6-6:
'@change' should be on a new line
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (6)
src/components/FwbFileInput/FwbFileInput.vue (6)
6-6: Fix the input element formatting and self-closing tag.The input element should not be self-closing, and its attributes should be on separate lines for better readability.
- <input :class="fileInpClasses" :multiple="multiple" type="file" :accept="accept" @change="handleChange" /> + <input + :class="fileInpClasses" + :multiple="multiple" + type="file" + :accept="accept" + @change="handleChange" + >Tools
GitHub Check: lint (18.x)
[warning] 6-6:
Disallow self-closing on HTML void elements ()
[warning] 6-6:
':multiple' should be on a new line
[warning] 6-6:
'type' should be on a new line
[warning] 6-6:
':accept' should be on a new line
[warning] 6-6:
'@change' should be on a new line
10-10: Fix the div element formatting.The attributes of the div element should be on separate lines for better readability.
- <div v-else class="flex flex-col items-start justify-center" @change="handleChange" @dragover="dragOverHandler" @drop="dropFileHandler"> + <div + v-else + class="flex flex-col items-start justify-center" + @change="handleChange" + @dragover="dragOverHandler" + @drop="dropFileHandler" + >Tools
11-11: Fix the span element formatting.The attributes of the span element should be on separate lines for better readability.
- <span v-if="label !== ''" :class="labelClasses">{{ label }}</span> + <span + v-if="label !== ''" + :class="labelClasses" + > + {{ label }} + </span>Tools
GitHub Check: lint (18.x)
[warning] 11-11:
':class' should be on a new line
14-14: Format the svg element attributes.For consistency, the attributes of the svg element should be on separate lines.
- <svg aria-hidden="true" class="w-8 h-8 text-gray-500 dark:text-gray-400" fill="none" viewBox="0 0 20 16" xmlns="http://www.w3.org/2000/svg"> + <svg + aria-hidden="true" + class="w-8 h-8 text-gray-500 dark:text-gray-400" + fill="none" + viewBox="0 0 20 16" + xmlns="http://www.w3.org/2000/svg" + >
32-32: Fix the hidden input element formatting and self-closing tag.The hidden input element should not be self-closing, and its attributes should be on separate lines for better readability.
- <input :multiple="multiple" type="file" :accept="accept" class="hidden" /> + <input + :multiple="multiple" + type="file" + :accept="accept" + class="hidden" + >
119-119: Format the destructuring of class variables.For consistency, the destructuring of class variables should be on separate lines.
- const { fileInpClasses, labelClasses, dropzoneLabelClasses, dropzoneWrapClasses, dropzoneTextClasses } = useFileInputClasses(props.size) + const { + fileInpClasses, + labelClasses, + dropzoneLabelClasses, + dropzoneWrapClasses, + dropzoneTextClasses + } = useFileInputClasses(props.size)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- src/components/FwbFileInput/FwbFileInput.vue (4 hunks)
Additional context used
GitHub Check: lint (18.x)
src/components/FwbFileInput/FwbFileInput.vue
[warning] 6-6:
Disallow self-closing on HTML void elements ()
[warning] 6-6:
':multiple' should be on a new line
[warning] 6-6:
'type' should be on a new line
[warning] 6-6:
':accept' should be on a new line
[warning] 6-6:
'@change' should be on a new line
[warning] 10-10:
'class' should be on a new line
[warning] 10-10:
'@change' should be on a new line
[warning] 10-10:
'@dragover' should be on a new line
[warning] 10-10:
'@drop' should be on a new line
[warning] 11-11:
':class' should be on a new line
|
@Sqrcz |
|
Ok, thank you. |
#292
Label was missing in FileInput's dropzone property, it was added
Summary by CodeRabbit
New Features
Bug Fixes
Refactor