feat: Label added for FileInput dropzone #304
Conversation
|
Important Review skippedMore than 25% of the files skipped due to max files limit. The review is being skipped to prevent a low-quality review. 390 files out of 603 files are above the max files limit of 200. You can disable this status message by setting the 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
🪧 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 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 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?
| @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.
| 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
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. |
|
Any updates on this one? It would be really great to have the label also being used for the dropzone. Or even better: use a slot for both. If needed and accepted I can create a PR for that. But I actually would need that quite soon if possible. |
|
Hey, @ps-20x I'll take a look at it gain before the end of the week... 🤞 |
|
Yeah, sorry... Wrong "end of the week" ;) I've drowned a bit in the move to tailwind V4. I'll take a look again today in the second half of the day. And let you know what we can do to move this one faster. |
@Sqrcz Sorry to bother again, but this would be great for something I would like to release ASAP. Any possibility to support you somewhere? |
|
Hey, to make it faster instead of asking for changes... I did all the formatting for you... (as well as sync with recent main) @ogzcode please update this PR by merging this one: https://github.com/ogzcode/flowbite-vue/pull/1 ;) And it will be good to go. 🙏 |
|
thank you too 🙏 |
4 participants
#292
Label was missing in FileInput's dropzone property, it was added
Summary by CodeRabbit
New Features
Bug Fixes
Refactor