-
Notifications
You must be signed in to change notification settings - Fork 12
fix(builder): replace live field previews in Add Fields palette #135
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
16d9e68
fix(builder): replace live previews with icon palette to kill autofill
harshtandiya 2745f62
test(e2e): cover Add Fields palette layout, search, and add-to-canvas
harshtandiya 9cbabe0
polish(builder): a11y + press feedback on Add Fields palette buttons
harshtandiya b0d23fa
polish(builder): tooltip on palette buttons explaining click action
harshtandiya e0975e4
polish(builder): fix scale transition and drop redundant tooltip on p…
harshtandiya f4375ed
polish(builder): a11y + semantic colors on sidebar tabs
harshtandiya File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,87 @@ | ||
| import { test, expect } from "../fixtures/test-data.fixture"; | ||
| import { FormBuilderPage } from "../helpers/form-builder"; | ||
|
|
||
| test.describe("Add Fields palette", () => { | ||
| test("renders palette items as buttons without live input previews", async ({ | ||
| page, | ||
| createForm, | ||
| }) => { | ||
| const formId = await createForm(); | ||
| const builder = new FormBuilderPage(page); | ||
| await builder.goto(formId); | ||
|
|
||
| const sidebar = page.locator( | ||
| '[data-form-builder-component="form-builder-sidebar"]' | ||
| ); | ||
|
|
||
| // Palette items are buttons, named after the fieldtype | ||
| await expect( | ||
| sidebar.getByRole("button", { name: "Data", exact: true }) | ||
| ).toBeVisible(); | ||
| await expect( | ||
| sidebar.getByRole("button", { name: "Email", exact: true }) | ||
| ).toBeVisible(); | ||
| await expect( | ||
| sidebar.getByRole("button", { name: "Phone", exact: true }) | ||
| ).toBeVisible(); | ||
|
|
||
| // No autofillable inputs inside palette buttons (regression: previously | ||
| // each palette card mounted a live FormControl, which triggered browser | ||
| // autofill on type=email / type=tel / type=password). | ||
| const emailBtn = sidebar.getByRole("button", { | ||
| name: "Email", | ||
| exact: true, | ||
| }); | ||
| await expect(emailBtn.locator("input")).toHaveCount(0); | ||
|
|
||
| const phoneBtn = sidebar.getByRole("button", { | ||
| name: "Phone", | ||
| exact: true, | ||
| }); | ||
| await expect(phoneBtn.locator("input")).toHaveCount(0); | ||
| }); | ||
|
|
||
| test("search filters palette items", async ({ page, createForm }) => { | ||
| const formId = await createForm(); | ||
| const builder = new FormBuilderPage(page); | ||
| await builder.goto(formId); | ||
|
|
||
| const sidebar = page.locator( | ||
| '[data-form-builder-component="form-builder-sidebar"]' | ||
| ); | ||
|
|
||
| await expect( | ||
| sidebar.getByRole("button", { name: "Data", exact: true }) | ||
| ).toBeVisible(); | ||
| await expect( | ||
| sidebar.getByRole("button", { name: "Phone", exact: true }) | ||
| ).toBeVisible(); | ||
|
|
||
| await sidebar.getByPlaceholder("Search Fields").fill("date"); | ||
|
|
||
| // "Date", "Date Time", "Date Range" remain (case-insensitive substring) | ||
| await expect( | ||
| sidebar.getByRole("button", { name: "Date", exact: true }) | ||
| ).toBeVisible(); | ||
| // Unrelated types are filtered out | ||
| await expect( | ||
| sidebar.getByRole("button", { name: "Phone", exact: true }) | ||
| ).toHaveCount(0); | ||
| await expect( | ||
| sidebar.getByRole("button", { name: "Data", exact: true }) | ||
| ).toHaveCount(0); | ||
| }); | ||
|
|
||
| test("clicking a palette item adds the field to the canvas", async ({ | ||
| page, | ||
| createForm, | ||
| }) => { | ||
| const formId = await createForm(); | ||
| const builder = new FormBuilderPage(page); | ||
| await builder.goto(formId); | ||
|
|
||
| await expect(builder.canvasEmptyState()).toBeVisible(); | ||
| await builder.addField("Email"); | ||
| await expect(builder.canvasEmptyState()).not.toBeVisible(); | ||
| }); | ||
| }); | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,56 +1,108 @@ | ||
| <script setup lang="ts"> | ||
| import { Settings, Plus, StretchHorizontal } from "@lucide/vue"; | ||
| import { ref } from "vue"; | ||
| import { computed, ref } from "vue"; | ||
| import { Tooltip } from "frappe-ui"; | ||
| import AddFieldsSection from "@/components/builder/sidebar/AddFieldsSection.vue"; | ||
| import SettingsSection from "@/components/builder/sidebar/SettingsSection.vue"; | ||
| import DocTypeFieldsSection from "@/components/builder/sidebar/DoctypeFieldsSection.vue"; | ||
|
|
||
| const sidebarSections = ref([ | ||
| const sidebarSections = [ | ||
| { | ||
| id: 0, | ||
| id: "settings", | ||
| label: "Settings", | ||
| icon: Settings, | ||
| section: SettingsSection, | ||
| }, | ||
| { | ||
| id: 1, | ||
| id: "add-fields", | ||
| label: "Add Fields", | ||
| icon: Plus, | ||
| section: AddFieldsSection, | ||
| }, | ||
| { | ||
| id: 2, | ||
| id: "doctype-fields", | ||
| label: "DocType Fields", | ||
| icon: StretchHorizontal, | ||
| section: DocTypeFieldsSection, | ||
| }, | ||
| ]); | ||
| ]; | ||
|
|
||
| const activeSection = ref(sidebarSections.value[1]); | ||
| const activeSection = ref( | ||
| sidebarSections.find((s) => s.id === "add-fields") ?? sidebarSections[0] | ||
| ); | ||
|
|
||
| const tabId = (id: string) => `form-builder-tab-${id}`; | ||
| const panelId = (id: string) => `form-builder-panel-${id}`; | ||
| const activeTabId = computed(() => tabId(activeSection.value.id)); | ||
| const activePanelId = computed(() => panelId(activeSection.value.id)); | ||
| </script> | ||
| <template> | ||
| <div | ||
| class="form-builder-sidebar bg-primary h-[calc(100vh-3rem)] w-72 border-r sticky top-0 overflow-y-auto flex" | ||
| class="form-builder-sidebar bg-surface-white h-[calc(100dvh-3rem)] w-72 border-r border-outline-gray-1 sticky top-0 overflow-y-auto flex" | ||
| data-form-builder-component="form-builder-sidebar" | ||
| > | ||
| <div class="h-full bg-inherit flex flex-col gap-2 p-2 border-r"> | ||
| <div | ||
| role="tablist" | ||
| aria-label="Form builder sections" | ||
| aria-orientation="vertical" | ||
| class="h-full bg-inherit flex flex-col gap-2 p-2 border-r border-outline-gray-1" | ||
| > | ||
| <Tooltip | ||
| v-for="section in sidebarSections" | ||
| :key="section.id" | ||
| :text="section.label" | ||
| placement="right" | ||
| > | ||
| <Button | ||
| size="md" | ||
| @click="activeSection = section" | ||
| :variant="activeSection === section ? 'subtle' : 'ghost'" | ||
| :icon="section.icon" | ||
| /> | ||
| <div class="relative"> | ||
| <span | ||
| v-if="activeSection.id === section.id" | ||
| aria-hidden="true" | ||
| class="absolute left-0 top-1/2 -translate-y-1/2 h-5 w-0.5 rounded-full bg-surface-gray-7" | ||
| /> | ||
| <Button | ||
| size="md" | ||
| role="tab" | ||
| :id="tabId(section.id)" | ||
| :aria-label="section.label" | ||
| :aria-selected="activeSection.id === section.id" | ||
| :aria-controls="panelId(section.id)" | ||
| @click="activeSection = section" | ||
| :variant="activeSection.id === section.id ? 'subtle' : 'ghost'" | ||
| :icon="section.icon" | ||
| /> | ||
| </div> | ||
| </Tooltip> | ||
| </div> | ||
| <div class="flex flex-col gap-4 w-full p-4 overflow-x-hidden"> | ||
| <component :is="activeSection.section" /> | ||
| <div | ||
| role="tabpanel" | ||
| :id="activePanelId" | ||
| :aria-labelledby="activeTabId" | ||
| tabindex="0" | ||
| class="flex flex-col gap-4 w-full p-4 overflow-x-hidden focus:outline-none" | ||
| > | ||
| <Transition name="section-fade" mode="out-in"> | ||
| <component :is="activeSection.section" :key="activeSection.id" /> | ||
| </Transition> | ||
| </div> | ||
| </div> | ||
| </template> | ||
|
|
||
| <style scoped> | ||
| .section-fade-enter-active { | ||
| transition: opacity 120ms ease-out; | ||
| } | ||
| .section-fade-leave-active { | ||
| transition: opacity 120ms ease-in; | ||
| } | ||
| .section-fade-enter-from, | ||
| .section-fade-leave-to { | ||
| opacity: 0; | ||
| } | ||
|
|
||
| @media (prefers-reduced-motion: reduce) { | ||
| .section-fade-enter-active, | ||
| .section-fade-leave-active { | ||
| transition: none; | ||
| } | ||
| } | ||
| </style> |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add assertions for Date Time and Date Range.
The comment on line 62 states that "Date", "Date Time", and "Date Range" should remain after filtering for "date", but only the "Date" button is verified. Add:
This ensures the filter correctly matches all date-related field types.
🤖 Prompt for AI Agents