left-sidebar #672
left-sidebar #672
Conversation
WalkthroughThis pull request introduces several changes to the client-side Vue components, primarily focusing on code formatting, layout restructuring, and the addition of new components. Key modifications include the introduction of a new 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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (8)
client/components/global/Header.vue (1)
2-2: Consider using design tokens for consistent styling.Hard-coded values like
h-[68px]should be moved to design tokens for better maintainability.client/components/pages/admin/UserWorkspaces.vue (1)
23-32: Consider extracting pagination logic to a computed property.The pagination visibility logic could be simplified and made more reusable.
+ const showPagination = computed(() => workspaces?.length > pageCount) <div - v-if="workspaces?.length > pageCount" + v-if="showPagination" class="flex justify-end px-3 py-3.5 border-t border-gray-200 dark:border-gray-700" >client/layouts/sidebar.vue (2)
26-52: Refactor navigation configuration for better maintainability.Consider these improvements:
- Extract navigation configs to a separate file
- Create a utility function for route params to avoid duplication
- Use constants for image paths
Example refactor:
// navigation.config.ts const createNavItem = (name: string, route: string, icon: string) => ({ name, route, params: { slug }, iconPath: `/img/sidebar/${icon}.svg` }) export const mainNavigation = [ createNavItem('Dashboard', 'home', 'dashboard'), createNavItem('Templates', 'templates-my-templates', 'templates'), createNavItem('Settings', 'settings-profile', 'settings') ] export const subNavigation = [ createNavItem('Submissions', 'forms-slug-show-submissions', 'submissions'), createNavItem('Analytics', 'forms-slug-show-stats', 'analytics'), createNavItem('Share', 'forms-slug-show-share', 'share') ] export const getIntegrationNav = () => createNavItem('Integrations', 'forms-slug-show-integrations', 'integrations')Also applies to: 54-83
61-70: Simplify conditional navigation items.The spread operator with conditional array makes the code harder to read. Consider using a more straightforward approach.
- ...(workspace.value.is_readonly - ? [] - : [ - { - name: "Integrations", - route: "forms-slug-show-integrations", - params: { slug }, - iconPath: "/img/sidebar/integrations.svg", - }, - ]), + if (!workspace.value.is_readonly) { + tabsList.push(createNavItem('Integrations', 'forms-slug-show-integrations', 'integrations')) + }client/components/global/SubSidebar.vue (2)
13-14: Consider using dynamic asset imports for better maintainability.Instead of hardcoding image paths, consider using dynamic imports or storing paths in a configuration file.
- <img - src="/img/sidebar/search.svg" - alt="icon" + <img + :src="require('@/assets/img/sidebar/search.svg')" + :alt="'Search icon'"
42-42: Use relative units for better responsiveness.Using pixel values for text sizes may not scale well across different screen sizes and user preferences.
- :class="[isSubSidebar ? 'text-sm' : 'text-[8px]']" + :class="[isSubSidebar ? 'text-sm' : 'text-xs']"client/pages/forms/[slug]/show.vue (1)
2-2: Use design tokens for colors instead of direct hex values.Replace hardcoded color values with design system tokens for better maintainability and consistency.
- <div class="bg-[#F5F7F9] w-full"> + <div class="bg-gray-50 w-full">client/pages/forms/[slug]/show/share.vue (1)
2-3: Consider using semantic class names for spacing.Instead of using raw pixel values for gaps, consider using Tailwind's spacing scale for consistency.
- <div class="w-full max-w-[1100px]"> - <div class="flex flex-col gap-3"> + <div class="w-full max-w-7xl"> + <div class="flex flex-col gap-4">
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (10)
client/public/img/sidebar/analytics.svgis excluded by!**/*.svgclient/public/img/sidebar/dashboard.svgis excluded by!**/*.svgclient/public/img/sidebar/help-center.svgis excluded by!**/*.svgclient/public/img/sidebar/integrations.svgis excluded by!**/*.svgclient/public/img/sidebar/plus.svgis excluded by!**/*.svgclient/public/img/sidebar/search.svgis excluded by!**/*.svgclient/public/img/sidebar/settings.svgis excluded by!**/*.svgclient/public/img/sidebar/share.svgis excluded by!**/*.svgclient/public/img/sidebar/submissions.svgis excluded by!**/*.svgclient/public/img/sidebar/templates.svgis excluded by!**/*.svg
📒 Files selected for processing (18)
client/components/forms/TextBlock.vue(1 hunks)client/components/forms/components/MentionDropdown.vue(1 hunks)client/components/forms/components/QuillyEditor.vue(1 hunks)client/components/global/ErrorBoundary.vue(1 hunks)client/components/global/Header.vue(1 hunks)client/components/global/SubSidebar.vue(1 hunks)client/components/open/forms/components/ShareFormUrl.vue(1 hunks)client/components/open/forms/fields/components/MatrixPrefilledValues.vue(1 hunks)client/components/pages/admin/DeletedForms.vue(1 hunks)client/components/pages/admin/UserWorkspaces.vue(2 hunks)client/components/pages/forms/show/EmbedCode.vue(3 hunks)client/components/pages/forms/show/ShareLink.vue(1 hunks)client/layouts/default.vue(1 hunks)client/layouts/sidebar.vue(1 hunks)client/pages/forms/[slug]/show.vue(3 hunks)client/pages/forms/[slug]/show/share.vue(1 hunks)client/pages/oauth/callback.vue(1 hunks)client/pages/update-credentials.vue(2 hunks)
✅ Files skipped from review due to trivial changes (11)
- client/components/pages/admin/DeletedForms.vue
- client/components/open/forms/fields/components/MatrixPrefilledValues.vue
- client/components/pages/forms/show/ShareLink.vue
- client/components/global/ErrorBoundary.vue
- client/components/forms/components/QuillyEditor.vue
- client/pages/oauth/callback.vue
- client/components/forms/TextBlock.vue
- client/pages/update-credentials.vue
- client/components/forms/components/MentionDropdown.vue
- client/components/pages/forms/show/EmbedCode.vue
- client/components/open/forms/components/ShareFormUrl.vue
🔇 Additional comments (1)
client/layouts/default.vue (1)
6-10: LGTM! Well-structured layout.The flex layout with max-width constraint and proper height classes provides a good foundation for responsive design.
| <div class="main-layout min-h-screen flex flex-col"> | ||
| <Navbar /> | ||
| <div class="main-layout min-h-screen flex flex-col h-full"> | ||
| <!-- <Navbar />--> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove commented-out code instead of keeping it.
Commented-out code should be removed rather than committed. If the code needs to be referenced later, it can be found in the git history.
- <!-- <Navbar />-->| <button | ||
| v-if="user" | ||
| class="bg-[#1C1D35] rounded w-6 h-6" | ||
| /> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add accessibility attributes and click handler to the button.
The button lacks essential accessibility attributes and a click handler.
<button
v-if="user"
- class="bg-[#1C1D35] rounded w-6 h-6"
+ class="bg-[#1C1D35] rounded w-6 h-6"
+ aria-label="Toggle navigation"
+ @click="handleClick"
/>Committable suggestion skipped: line range outside the PR's diff.
| // { | ||
| // name: "Help Center", | ||
| // route: opnformConfig.links.help_url, | ||
| // params: { slug }, | ||
| // iconPath: "/img/sidebar/help-center.svg", | ||
| // }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove commented-out code for help center.
Commented-out code should be removed from the codebase.
- // {
- // name: "Help Center",
- // route: opnformConfig.links.help_url,
- // params: { slug },
- // iconPath: "/img/sidebar/help-center.svg",
- // },| <img | ||
| alt="icon" | ||
| :src="tab.iconPath" | ||
| :class="{ | ||
| 'filter-none': $route.name === tab.route, | ||
| grayscale: $route.name !== tab.route, | ||
| }" | ||
| > |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance accessibility with descriptive alt text.
The current alt text "icon" is not descriptive enough for screen readers.
<img
- alt="icon"
+ :alt="`${tab.name} icon`"
:src="tab.iconPath"
:class="{
'filter-none': $route.name === tab.route,
grayscale: $route.name !== tab.route,
}"
>📝 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.
| <img | |
| alt="icon" | |
| :src="tab.iconPath" | |
| :class="{ | |
| 'filter-none': $route.name === tab.route, | |
| grayscale: $route.name !== tab.route, | |
| }" | |
| > | |
| <img | |
| :alt="`${tab.name} icon`" | |
| :src="tab.iconPath" | |
| :class="{ | |
| 'filter-none': $route.name === tab.route, | |
| grayscale: $route.name !== tab.route, | |
| }" | |
| > |
| const props = defineProps({ | ||
| tabsList: { type: Array, default: null }, | ||
| isSubSidebar: { type: Boolean, default: false }, | ||
| }) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Strengthen prop definitions for better type safety.
Consider improving the props definition:
- Make tabsList required since it's essential for rendering
- Use an empty array as default instead of null
- Add validation for the tabsList items structure
const props = defineProps({
- tabsList: { type: Array, default: null },
+ tabsList: {
+ type: Array,
+ required: true,
+ validator: (value) => value.every(tab =>
+ ['name', 'route', 'iconPath'].every(key => key in tab)
+ )
+ },
isSubSidebar: { type: Boolean, default: false },
})📝 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.
| const props = defineProps({ | |
| tabsList: { type: Array, default: null }, | |
| isSubSidebar: { type: Boolean, default: false }, | |
| }) | |
| const props = defineProps({ | |
| tabsList: { | |
| type: Array, | |
| required: true, | |
| validator: (value) => value.every(tab => | |
| ['name', 'route', 'iconPath'].every(key => key in tab) | |
| ) | |
| }, | |
| isSubSidebar: { type: Boolean, default: false }, | |
| }) |
| <!-- <UButton--> | ||
| <!-- v-if="!workspace.is_readonly"--> | ||
| <!-- v-track.edit_form_click="{--> | ||
| <!-- form_id: form.id,--> | ||
| <!-- form_slug: form.slug,--> | ||
| <!-- }"--> | ||
| <!-- color="primary"--> | ||
| <!-- icon="i-heroicons-pencil"--> | ||
| <!-- class="hover:no-underline"--> | ||
| <!-- :to="{ name: 'forms-slug-edit', params: { slug: form.slug } }"--> | ||
| <!-- >--> | ||
| <!-- Edit <span class="hidden md:inline">form</span>--> | ||
| <!-- </UButton>--> | ||
| <!-- <extra-menu v-if="!workspace.is_readonly" :form="form" />--> | ||
| <!-- </div>--> | ||
| <!-- </div>--> | ||
|
|
||
| <!-- <p class="text-gray-500 text-sm">--> | ||
| <!-- <span class="pr-1"--> | ||
| <!-- >{{ form.views_count }} view{{--> | ||
| <!-- form.views_count > 0 ? "s" : ""--> | ||
| <!-- }}</span--> | ||
| <!-- >--> | ||
| <!-- <span class="pr-1"--> | ||
| <!-- >- {{ form.submissions_count }} submission{{--> | ||
| <!-- form.submissions_count > 0 ? "s" : ""--> | ||
| <!-- }}--> | ||
| <!-- </span>--> | ||
| <!-- <span>- Edited {{ form.last_edited_human }}</span>--> | ||
| <!-- </p>--> | ||
| <!-- <div--> | ||
| <!-- v-if="--> | ||
| <!-- ['draft', 'closed'].includes(form.visibility) ||--> | ||
| <!-- (form.tags && form.tags.length > 0)--> | ||
| <!-- "--> | ||
| <!-- class="mt-2 flex items-center flex-wrap gap-3"--> | ||
| <!-- >--> | ||
| <!-- <span--> | ||
| <!-- v-if="form.visibility == 'draft'"--> | ||
| <!-- class="inline-flex items-center rounded-full bg-yellow-100 px-2 py-1 text-xs font-medium text-yellow-600 ring-1 ring-inset ring-gray-500/10 dark:text-white dark:bg-gray-700"--> | ||
| <!-- >--> | ||
| <!-- Draft - not publicly accessible--> | ||
| <!-- </span>--> | ||
| <!-- <span--> | ||
| <!-- v-else-if="form.visibility == 'closed'"--> | ||
| <!-- class="inline-flex items-center rounded-full bg-yellow-100 px-2 py-1 text-xs font-medium text-yellow-600 ring-1 ring-inset ring-gray-500/10 dark:text-white dark:bg-gray-700"--> | ||
| <!-- >--> | ||
| <!-- Closed - won't accept new submissions--> | ||
| <!-- </span>--> | ||
| <!-- <span--> | ||
| <!-- v-for="tag in form.tags"--> | ||
| <!-- :key="tag"--> | ||
| <!-- class="inline-flex items-center rounded-full bg-gray-50 px-2 py-1 text-xs font-medium text-gray-600 ring-1 ring-inset ring-gray-500/10 dark:text-white dark:bg-gray-700"--> | ||
| <!-- >--> | ||
| <!-- {{ tag }}--> | ||
| <!-- </span>--> | ||
| <!-- </div>--> | ||
|
|
||
| <!-- <p v-if="form.closes_at" class="text-yellow-500">--> | ||
| <!-- <span v-if="form.is_closed">--> | ||
| <!-- This form stopped accepting submissions on the--> | ||
| <!-- {{ displayClosesDate }}--> | ||
| <!-- </span>--> | ||
| <!-- <span v-else>--> | ||
| <!-- This form will stop accepting submissions on the--> | ||
| <!-- {{ displayClosesDate }}--> | ||
| <!-- </span>--> | ||
| <!-- </p>--> | ||
| <!-- <p v-if="form.max_submissions_count > 0" class="text-yellow-500">--> | ||
| <!-- <span v-if="form.max_number_of_submissions_reached">--> | ||
| <!-- The form is now closed because it reached its limit of--> | ||
| <!-- {{ form.max_submissions_count }} submissions.--> | ||
| <!-- </span>--> | ||
| <!-- <span v-else>--> | ||
| <!-- This form will stop accepting submissions after--> | ||
| <!-- {{ form.max_submissions_count }} submissions.--> | ||
| <!-- </span>--> | ||
| <!-- </p>--> | ||
|
|
||
| <!-- <form-cleanings class="mt-4" :form="form" />--> | ||
|
|
||
| <!-- <div class="border-b border-gray-200 dark:border-gray-700">--> | ||
| <!-- <ul class="flex flex-wrap -mb-px text-sm font-medium text-center">--> | ||
| <!-- <li v-for="(tab, i) in tabsList" :key="i + 1" class="mr-6">--> | ||
| <!-- <nuxt-link--> | ||
| <!-- :to="{ name: tab.route, params: tab.params ?? {} }"--> | ||
| <!-- class="hover:no-underline inline-block py-4 rounded-t-lg border-b-2 text-gray-500 hover:text-gray-600"--> | ||
| <!-- active-class="text-blue-600 hover:text-blue-900 dark:text-blue-500 dark:hover:text-blue-500 border-blue-600 dark:border-blue-500"--> | ||
| <!-- >--> | ||
| <!-- {{ tab.name }}--> | ||
| <!-- </nuxt-link>--> | ||
| <!-- </li>--> | ||
| <!-- </ul>--> | ||
| <!-- </div>--> | ||
| <!-- </div>--> | ||
| <!-- </div>--> | ||
| <!-- </div>--> | ||
| <!-- <div class="flex flex-col bg-white">--> | ||
| <!-- <NuxtPage :form="form" />--> | ||
| <!-- </div>--> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove commented code blocks.
Large blocks of commented code should be removed as they:
- Add unnecessary file size
- Make the code harder to maintain
- Can cause confusion about which implementation is current
Consider using version control to track these changes instead of keeping commented code.
| <!-- <div class="mb-6 pb-6 border-b w-full flex flex-col sm:flex-row gap-2">--> | ||
| <!-- <regenerate-form-link--> | ||
| <!-- v-if="!workspace.is_readonly"--> | ||
| <!-- class="sm:w-1/2 flex"--> | ||
| <!-- :form="props.form"--> | ||
| <!-- />--> | ||
|
|
||
| <url-form-prefill | ||
| class="sm:w-1/2" | ||
| :form="props.form" | ||
| :extra-query-param="shareUrlForQueryParams" | ||
| /> | ||
| <!-- <url-form-prefill--> | ||
| <!-- class="sm:w-1/2"--> | ||
| <!-- :form="props.form"--> | ||
| <!-- :extra-query-param="shareUrlForQueryParams"--> | ||
| <!-- />--> | ||
|
|
||
| <embed-form-as-popup-modal | ||
| class="sm:w-1/2 flex" | ||
| :form="props.form" | ||
| /> | ||
| </div> | ||
| <!-- <embed-form-as-popup-modal--> | ||
| <!-- class="sm:w-1/2 flex"--> | ||
| <!-- :form="props.form"--> | ||
| <!-- />--> | ||
| <!-- </div>--> | ||
|
|
||
| <share-link | ||
| class="mt-4" | ||
| :form="props.form" | ||
| :extra-query-param="shareUrlForQueryParams" | ||
| /> | ||
|
|
||
| <embed-code | ||
| class="mt-6" | ||
| :form="props.form" | ||
| :extra-query-param="shareUrlForQueryParams" | ||
| /> | ||
|
|
||
| <form-qr-code | ||
| class="mt-6" | ||
| :form="props.form" | ||
| :extra-query-param="shareUrlForQueryParams" | ||
| /> | ||
| <!-- <form-qr-code--> | ||
| <!-- class="mt-6"--> | ||
| <!-- :form="props.form"--> | ||
| <!-- :extra-query-param="shareUrlForQueryParams"--> | ||
| <!-- />--> | ||
|
|
||
| <advanced-form-url-settings | ||
| v-model="shareFormConfig" | ||
| :form="props.form" | ||
| /> | ||
| <!-- <advanced-form-url-settings--> | ||
| <!-- v-model="shareFormConfig"--> | ||
| <!-- :form="props.form"--> | ||
| <!-- />--> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove commented component blocks and their corresponding imports.
Several components are commented out in the template but their imports are still present. Clean up the code by:
- Removing the commented component blocks
- Removing unused imports
-import FormQrCode from "~/components/pages/forms/show/FormQrCode.vue"
-import UrlFormPrefill from "~/components/pages/forms/show/UrlFormPrefill.vue"
-import RegenerateFormLink from "~/components/pages/forms/show/RegenerateFormLink.vue"
-import AdvancedFormUrlSettings from "~/components/open/forms/components/AdvancedFormUrlSettings.vue"
-import EmbedFormAsPopupModal from "~/components/pages/forms/show/EmbedFormAsPopupModal.vue"📝 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 class="mb-6 pb-6 border-b w-full flex flex-col sm:flex-row gap-2">--> | |
| <!-- <regenerate-form-link--> | |
| <!-- v-if="!workspace.is_readonly"--> | |
| <!-- class="sm:w-1/2 flex"--> | |
| <!-- :form="props.form"--> | |
| <!-- />--> | |
| <url-form-prefill | |
| class="sm:w-1/2" | |
| :form="props.form" | |
| :extra-query-param="shareUrlForQueryParams" | |
| /> | |
| <!-- <url-form-prefill--> | |
| <!-- class="sm:w-1/2"--> | |
| <!-- :form="props.form"--> | |
| <!-- :extra-query-param="shareUrlForQueryParams"--> | |
| <!-- />--> | |
| <embed-form-as-popup-modal | |
| class="sm:w-1/2 flex" | |
| :form="props.form" | |
| /> | |
| </div> | |
| <!-- <embed-form-as-popup-modal--> | |
| <!-- class="sm:w-1/2 flex"--> | |
| <!-- :form="props.form"--> | |
| <!-- />--> | |
| <!-- </div>--> | |
| <share-link | |
| class="mt-4" | |
| :form="props.form" | |
| :extra-query-param="shareUrlForQueryParams" | |
| /> | |
| <embed-code | |
| class="mt-6" | |
| :form="props.form" | |
| :extra-query-param="shareUrlForQueryParams" | |
| /> | |
| <form-qr-code | |
| class="mt-6" | |
| :form="props.form" | |
| :extra-query-param="shareUrlForQueryParams" | |
| /> | |
| <!-- <form-qr-code--> | |
| <!-- class="mt-6"--> | |
| <!-- :form="props.form"--> | |
| <!-- :extra-query-param="shareUrlForQueryParams"--> | |
| <!-- />--> | |
| <advanced-form-url-settings | |
| v-model="shareFormConfig" | |
| :form="props.form" | |
| /> | |
| <!-- <advanced-form-url-settings--> | |
| <!-- v-model="shareFormConfig"--> | |
| <!-- :form="props.form"--> | |
| <!-- />--> | |
| <share-link | |
| :form="props.form" | |
| :extra-query-param="shareUrlForQueryParams" | |
| /> | |
| <embed-code | |
| :form="props.form" | |
| :extra-query-param="shareUrlForQueryParams" | |
| /> |
Summary by CodeRabbit
Release Notes
New Components
Headercomponent to improve app navigationSubSidebarcomponent for enhanced sidebar functionalitysidebar.vuelayout for improved navigation structureLayout Updates
HeaderandSidebarStyle Improvements
Minor Refinements