Design improvements for Shoppy (Part 1) #80

heypoom · 2024-11-01T11:04:13Z

Closes #79

This PR makes design improvements to Shoppy. Refer to the product docs for the full list of changes to Shoppy - this PR acts on a subset of them, as other changes will require changes to the SDK itself. I have commented in the product doc on which change requires further improvements.

PS. the diffs is a bit misleading, the yarn.lock changes make it looks big but it's actually not a lot of diffs.

Loom

Private Loom

Checklist

Sidebar navigation copy
Remove the divider from sidebar
Add "New custom exploration" and "New dashboard" buttons
Small copy tweaks
Implement the "add a filter" and "change the summary" buttons by using modular InteractiveQuestion components
Re-implement the “create question from template” flow with InteractiveQuestion instead of ModifyQuestion

vercel · 2024-11-01T11:04:16Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
customer-zero	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Nov 4, 2024 0:00am

heypoom · 2024-11-01T17:36:40Z

src/components/CustomIcon.tsx

+const icons = {
+  filter:
+    "M1 4.5c0-.69.56-1.25 1.25-1.25h11.5a1.25 1.25 0 1 1 0 2.5H2.25C1.56 5.75 1 5.19 1 4.5zM4 9a1 1 0 0 1 1-1h6a1 1 0 1 1 0 2H5a1 1 0 0 1-1-1zm2 4.25a1 1 0 0 1 1-1h2a1 1 0 1 1 0 2H7a1 1 0 0 1-1-1z",
+  sum: "M2.295 2.745A.75.75 0 0 1 3 2.25h9a.75.75 0 1 1 0 1.5H5.072l3.486 2.906c.84.7.84 1.989 0 2.688L5.072 12.25h6.763a.75.75 0 0 1 0 1.5H3a.75.75 0 0 1-.48-1.326l5.078-4.232a.25.25 0 0 0 0-.384L2.52 3.576a.75.75 0 0 1-.225-.831z",
+  notebook:
+    "M3.5 3a.5.5 0 0 1 .5.5V5a.5.5 0 0 1-.5.5H2a.5.5 0 0 1-.5-.5V3.5A.5.5 0 0 1 2 3h1.5zM11.5 3.5A.5.5 0 0 0 11 3H6a.5.5 0 0 0-.5.5V5a.5.5 0 0 0 .5.5h5a.5.5 0 0 0 .5-.5V3.5zM14.5 7.25a.5.5 0 0 0-.5-.5H6a.5.5 0 0 0-.5.5v1.5a.5.5 0 0 0 .5.5h8a.5.5 0 0 0 .5-.5v-1.5zM4 7.25a.5.5 0 0 0-.5-.5H2a.5.5 0 0 0-.5.5v1.5a.5.5 0 0 0 .5.5h1.5a.5.5 0 0 0 .5-.5v-1.5zM3.5 10.5a.5.5 0 0 1 .5.5v1.5a.5.5 0 0 1-.5.5H2a.5.5 0 0 1-.5-.5V11a.5.5 0 0 1 .5-.5h1.5zM10 11a.5.5 0 0 0-.5-.5H6a.5.5 0 0 0-.5.5v1.5a.5.5 0 0 0 .5.5h3.5a.5.5 0 0 0 .5-.5V11z",
+} as const satisfies Record<string, string>


The icons used in Maz's design are Metabase icons, so we add the icon paths here.

heypoom · 2024-11-01T17:37:02Z

package.json

@@ -7,7 +7,7 @@
    "@mantine/core": "^7.8.0",
    "@mantine/form": "^7.8.0",
    "@mantine/hooks": "^7.8.0",
-    "@metabase/embedding-sdk-react": "^0.1.37",
+    "@metabase/embedding-sdk-react": "^1.51.2",


Targets the 51-stable version.

heypoom · 2024-11-01T17:38:08Z

src/components/InteractiveQuestionView.tsx

+type QuestionView = "viz" | "filter" | "summary" | "editor"
+
+interface Props {
+  isSaveEnabled?: boolean


Currently the save question button is only enabled in the "New Question from Template" flow.

heypoom · 2024-11-01T17:38:55Z

src/components/InteractiveQuestionView.tsx

+          withCloseButton={false}
+        >
+          <Box bg="background">
+            <InteractiveQuestion.SaveQuestionForm onClose={closeSaveModal} />


The form only allows creating new questions, and not "save as" as far as I am aware.

Is there a way to automatically close the modal once the question is saved? mine showed success then changed to save again after a while, but the modal remains which is not expected.

@WiNloSt This smells like a bug on the SDK - the InteractiveQuestion.SaveQuestionForm component only exposes a single onClose prop, so if the modal does not close after success we need to fix it. Let me re-test it.

heypoom · 2024-11-04T11:51:30Z

src/components/CustomIcon.tsx

+  ...props
+}: CustomIconProps) => (
+  <svg
+    viewBox="0 0 16 16"


So far our icons here only uses the 16x16 viewBox.

heypoom · 2024-11-04T11:52:02Z

src/components/InteractiveQuestionView.tsx

+    (view: QuestionView, nextView: QuestionView) =>
+      view !== "viz" && view === nextView ? "viz" : nextView,


This allows us to click on the same view's button again and go back to the visualization view.

inlining this logic makes it a little hard to understand. What this does is essentially changing the view to viz if we're clicking the same button as the current view. Otherwise, change to whatever the nextViz is.

I'd like to see this being structured with some if blocks, that might make this more readable.

@WiNloSt Good idea! I initially has this explicitly written with if-blocks, but later collapsed to this. I'll revert this then, as the former version is indeed more readable.

heypoom · 2024-11-04T11:52:26Z

src/components/InteractiveQuestionView.tsx

+}
+
+export const InteractiveQuestionView = (props: Props) => {
+  const { isSaveEnabled = false } = props


We only enable the "Save" button in the "new question from template" page for now.

why do you destructure props on some components but not all? I can understand in case you need to access the whole props for spreading for example. But I don't see the reason to do that here. I found using props needs a bit more work to figure out where the all the props are used.

heypoom · 2024-11-04T11:53:32Z

src/components/InteractiveQuestionView.tsx

+      </Flex>
+
+      {view === "viz" && (
+        <Box h="500px">


We use a fixed height for now, as the SDK does not provide a sensible default height in QuestionVisualization yet. We can remove this from Shoppy once the SDK implements a sane default height in QuestionVisualization.

heypoom · 2024-11-04T12:00:01Z

src/store/site.ts

@@ -13,3 +14,5 @@ export const siteAtom = atomWithStorage<SiteKey>(
    getOnInit: true,
  },
 )
+
+export const siteIsReloadingAtom = atom(false)


We need to fully reload the page in some cases - do a full page search on useReloadOnSiteChange to see where that applies. This helps us to show the full-page loading indicator while the page is being reloaded by window.location.reload()

WiNloSt

Tested and all looks good 👍

WiNloSt · 2024-11-05T10:07:03Z

src/components/InteractiveQuestionView.tsx

+}
+
+export const InteractiveQuestionView = (props: Props) => {
+  const { isSaveEnabled = false } = props


why do you destructure props on some components but not all? I can understand in case you need to access the whole props for spreading for example. But I don't see the reason to do that here. I found using props needs a bit more work to figure out where the all the props are used.

WiNloSt · 2024-11-05T10:15:10Z

src/components/InteractiveQuestionView.tsx

+        rowGap="sm"
+      >
+        <Group gap="xs">
+          <InteractiveQuestion.BackButton />


How do you get this button to render? I don't couldn't figure out how.

WiNloSt · 2024-11-05T10:17:43Z

src/components/InteractiveQuestionView.tsx

+    (view: QuestionView, nextView: QuestionView) =>
+      view !== "viz" && view === nextView ? "viz" : nextView,


inlining this logic makes it a little hard to understand. What this does is essentially changing the view to viz if we're clicking the same button as the current view. Otherwise, change to whatever the nextViz is.

I'd like to see this being structured with some if blocks, that might make this more readable.

WiNloSt · 2024-11-05T10:19:53Z

src/components/InteractiveQuestionView.tsx

+          withCloseButton={false}
+        >
+          <Box bg="background">
+            <InteractiveQuestion.SaveQuestionForm onClose={closeSaveModal} />


Is there a way to automatically close the modal once the question is saved? mine showed success then changed to save again after a while, but the modal remains which is not expected.

heypoom added 7 commits November 1, 2024 16:23

sidebar copy changes

027538c

add themed button

c7722cb

forward refs

781e5a1

bump sdk version

c7146c0

replace ModifyQuestion with InteractiveQuestion

8085f06

add custom interactive question view skeleton

c0e776f

add placeholder view

509e239

vercel bot deployed to Preview November 1, 2024 11:04 View deployment

heypoom added the enhancement New feature or request label Nov 1, 2024

heypoom self-assigned this Nov 1, 2024

add custom view components

9489e4a

vercel bot deployed to Preview November 1, 2024 15:33 View deployment

add more interactive question views

6f51045

vercel bot deployed to Preview November 1, 2024 15:49 View deployment

button visual tweaks

a9e1798

vercel bot deployed to Preview November 1, 2024 16:33 View deployment

add save button

3dddc37

vercel bot deployed to Preview November 1, 2024 16:43 View deployment

add save modal

dcd0dd9

vercel bot deployed to Preview November 1, 2024 16:49 View deployment

responsive styles

f2b90a7

vercel bot deployed to Preview November 1, 2024 16:57 View deployment

heypoom marked this pull request as ready for review November 1, 2024 17:32

heypoom changed the title ~~UX improvements for Shoppy (Part 1)~~ Visual improvements for Shoppy (Part 1) Nov 1, 2024

heypoom changed the title ~~Visual improvements for Shoppy (Part 1)~~ Design improvements for Shoppy (Part 1) Nov 1, 2024

improve reload flow

7e22241

vercel bot deployed to Preview November 1, 2024 17:43 View deployment

icon color tweaks

77bef86

vercel bot deployed to Preview November 4, 2024 09:10 View deployment

refactor to useReducer

73e5e18

vercel bot deployed to Preview November 4, 2024 11:19 View deployment

add missing new dashboard link

6464cbe

vercel bot deployed to Preview November 4, 2024 11:48 View deployment

do not wrap FullPageLoader

9fee562

vercel bot deployed to Preview November 4, 2024 11:56 View deployment

revert withCardTitle prop

3e17b61

vercel bot deployed to Preview November 4, 2024 12:00 View deployment

heypoom commented Nov 4, 2024

View reviewed changes

heypoom requested a review from a team November 4, 2024 12:01

WiNloSt approved these changes Nov 5, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Design improvements for Shoppy (Part 1) #80

Design improvements for Shoppy (Part 1) #80

heypoom commented Nov 1, 2024 •

edited

Loading

vercel bot commented Nov 1, 2024 •

edited

Loading

heypoom Nov 1, 2024

heypoom Nov 1, 2024

heypoom Nov 1, 2024

heypoom Nov 1, 2024

WiNloSt Nov 5, 2024

heypoom Nov 5, 2024

heypoom Nov 4, 2024

heypoom Nov 4, 2024

WiNloSt Nov 5, 2024

heypoom Nov 5, 2024 •

edited

Loading

heypoom Nov 4, 2024

WiNloSt Nov 5, 2024

heypoom Nov 4, 2024

heypoom Nov 4, 2024

WiNloSt left a comment

WiNloSt Nov 5, 2024

WiNloSt Nov 5, 2024

WiNloSt Nov 5, 2024

WiNloSt Nov 5, 2024

		(view: QuestionView, nextView: QuestionView) =>
		view !== "viz" && view === nextView ? "viz" : nextView,

Design improvements for Shoppy (Part 1) #80

Are you sure you want to change the base?

Design improvements for Shoppy (Part 1) #80

Conversation

heypoom commented Nov 1, 2024 • edited Loading

Loom

Checklist

vercel bot commented Nov 1, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

heypoom Nov 5, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

WiNloSt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

heypoom commented Nov 1, 2024 •

edited

Loading

vercel bot commented Nov 1, 2024 •

edited

Loading

heypoom Nov 5, 2024 •

edited

Loading