adds minimal embed focused homepage #38402

npretto · 2024-02-02T17:12:04Z

[Epic] Meet embedders at the door - minimal edition

Description

This adds a the minimal homepage from this design.
For the tests, I only modified a current e2e to expect to not find the homepage, and then I made a new flow that selects "Interested in embedding" and expects to find it.

I will add more tests in a later PR

How to verify

Do the setup, select that you're interested in embedding or "both"
You should see the embedding homepage
You should be able to dismiss it and it should go away and never come back again

Demo

Checklist

Tests have been added/updated to cover changes in this PR

npretto · 2024-02-05T15:51:55Z

frontend/src/metabase/home/components/EmbedMinimalHomepage/util.ts

+  try {
+    localStorage.setItem("showEmbedHomepage", "true");
+  } catch (e) {
+    console.error(e);


localStorage could fail (if disabled, or if in incognito in some browsers).
I still want to see the error if it happens but I don't want to trigger ErrorBoundaries

npretto · 2024-02-05T16:04:03Z

frontend/src/metabase/home/components/HomeContent/HomeContent.tsx

@@ -26,6 +31,10 @@ export const HomeContent = (): JSX.Element | null => {
    return <LoadingAndErrorWrapper loading />;
  }

+  if (isAdmin && shouldSHowEmbedHomepage()) {
+    return <EmbedMinimalHomepage onDismiss={update} />;


onDismiss={update} is needed because we use a value from the local storage and we need to trigger a rerender to make the UI update.
I tried to use useLocalStorage from react-use but that's also not reactive.

I wouldn't worry too much about this at the moment, as this behaviour is covered by a e2e and in v50 we'll render this homepage based on a setting from the BE, removing the local storage flag

npretto · 2024-02-05T16:54:19Z

e2e/test/scenarios/onboarding/setup/setup.cy.spec.js

+  it("embedded use-case, it should hide the db step and show the embedding homepage", () => {
+    cy.visit("/setup");
+
+    cy.location("pathname").should("eq", "/setup");


this is a copy and paste of another test, I will refactor them later to extract common bits, for now I wanted to get this merged/have feedback earlier

github-actions · 2024-02-05T17:01:27Z

Codenotify: Notifying subscribers in CODENOTIFY files for diff 0d63201...bdb803d.

Notify	File(s)
@ranquild	frontend/src/metabase/home/components/EmbedMinimalHomepage/EmbedMinimalHomepage.styled.tsx frontend/src/metabase/home/components/EmbedMinimalHomepage/EmbedMinimalHomepage.tsx frontend/src/metabase/home/components/EmbedMinimalHomepage/index.ts frontend/src/metabase/home/components/HomeContent/HomeContent.tsx frontend/src/metabase/home/utils.ts frontend/src/metabase/setup/actions.ts frontend/src/metabase/setup/setup.unit.spec.tsx

replay-io · 2024-02-05T17:13:23Z

Status	Complete ↗︎
Commit	`bdb803d`
Results	⚠️ 2 Flaky ✅ 2254 Passed

npretto · 2024-02-06T10:15:12Z

frontend/src/metabase/setup/actions.ts

+      if (usageReason === "embedding" || usageReason === "both") {
+        setShowEmbedHomepageFlag();
+      } else {
+        // make sure that state is clean in case of more than one setup on the same browser


Vitorio wasn't able to test the "normal" cases after he triggered the homepage, because he probably didn't dismissed it and the flag stayed in localStorage in the following setups.
Multiple setups on the same browser is a pretty rare edge case but the fix is just cleaning the state

WiNloSt

I have minor comments, otherwise it looks good to me 👍

WiNloSt · 2024-02-07T13:07:24Z

e2e/test/scenarios/onboarding/setup/setup.cy.spec.js

+      .findByText("Get started with Embedding Metabase in your app")
+      .should("exist");
+
+    cy.icon("close").click();


before we close it can you add a reload here to ensure everything works before you dismiss it?

Very good point, added it in 2c75c42

WiNloSt · 2024-02-07T13:20:45Z

frontend/src/metabase/home/components/HomeContent/HomeContent.tsx

@@ -14,9 +15,13 @@ import { HomeRecentSection } from "../HomeRecentSection";
 import { HomeXraySection } from "../HomeXraySection";
 import { getIsXrayEnabled } from "../../selectors";
 import { isWithinWeeks } from "../../utils";
+import { shouldSHowEmbedHomepage } from "../EmbedMinimalHomepage/util";


I'm not sure this util should be imported from a file under another component. Maybe it would be better to move the util to somewhere else more generic e.g. frontend/src/metabase/home/utils.ts.

Also

Suggested change

import { shouldSHowEmbedHomepage } from "../EmbedMinimalHomepage/util";

import { shouldShowEmbedHomepage } from "../EmbedMinimalHomepage/util";

Moved all the utils to /home/utils.ts 👍 e97cde2

WiNloSt · 2024-02-07T13:24:52Z

frontend/src/metabase/home/components/EmbedMinimalHomepage/EmbedMinimalHomepage.tsx

+export const EmbedMinimalHomepage = ({
+  onDismiss,
+}: EmbedMinimalHomepageProps) => {
+  const learnMoreUrl = useSelector(s =>


Please try to refrain from using abbreviated names, and use the full name for clarity instead.

Suggested change

const learnMoreUrl = useSelector(s =>

const learnMoreUrl = useSelector(state =>

👍 1556490

WiNloSt · 2024-02-07T13:28:36Z

frontend/src/metabase/home/components/EmbedMinimalHomepage/EmbedMinimalHomepage.tsx

+            </Link>
+
+            <Text>{jt`${(
+              <ExternalLink href={learnMoreUrl}>{t`Learn more`}</ExternalLink>


Components inside jt needs to have key because jt will convert these to array as far as I understand.

This is the error from this component.

I keep forgetting about it 🤦 bdb803d

* refactor: prepare SettingsPage to have conditionally rendered steps (#38201) * refactor: prepare SettingsPage to have conditionally rendered steps * fix tests missing props * Usage reason question (#38267) * Invactive -> Inactive * add question * fix e2e test * small refactor * Update frontend/src/metabase/setup/components/UsageQuestionStep/UsageQuestionStep.tsx Co-authored-by: Mahatthana (Kelvin) Nomsawadi <me@bboykelvin.dev> --------- Co-authored-by: Mahatthana (Kelvin) Nomsawadi <me@bboykelvin.dev> * Hide "Add your data" step for people interested in embedding (#38390) * hide 'Add your data' step when user is interested in embedding * add other test cases * refactor: create selector * address pr feedback * adds minimal embed focused homepage (#38402) * very minimal homepage * fixed text color * minimal e2e + actual logic that i forgot lol * refactor the list to not have the numbers in the content * rename util functions * use button to have cursor when hovering + accessibility * refactor: move the waitFor inside the submit step * fix: make sure state is clean on multiple installs in a row * test: check that the homepage persists reload + removed typos * rename s to state * moved util functions up to home/utils.ts * adds key to avoid react warning * refactor and tests for [meet embedders] (#38465) * test: check all numbered steps during the setup flow * refactor: introduce getSteps and start using the step name instead of a number to identify steps * refactor: remove map of the steps, use step names directly * refactor: getNextStep * include 'hidden' steps in getSteps so that it won't mess up with analytics * fix after rebase issue * use full name instead of s, s -> step * analytics for meet-embedders (#38521) * refactor: extract schema version and onboarding version into constants * remove old ga events * trackUsageReasonSelected * update trackStepSeen to send the correct index * copy old schema for better diff * adds new snowplow event for usage_reason step * schema and e2e update for usage_reason_selected * adds utm tag to the link in the minimal embed homepage --------- Co-authored-by: Mahatthana (Kelvin) Nomsawadi <me@bboykelvin.dev>

* serialization: snowplow events for export/import (#38487) * Dashboard sections (#38463) * Add `ADD_MANY_CARDS_TO_DASH` action * Reuse `NewDashCardOpts` type * Extract `DashboardCardLayoutAttrs` type * Add basic `DashCardPlaceholder` viz * Add section layouts * Add `addSectionToDashboard` redux action * Add `SectionLayoutPreview` component * Add "Add section" button to dashboard header * Allow placeholder cards in `replaceCard` * Make heading cards transparent by default * Fix layout fn usage * Update mapping UI for placeholder cards * Fix tooltips position * Add `createMockPlaceholderDashboardCard` * Test `replaceCard` works with placeholder cards * Extract common `setup` from cards actions tests * Add `createMockDashboardTab` * Add unit tests for `addSectionToDashboard` * Placeholder cards work for emails/pulses Dashboards with placeholder cards now properly email rather than throwing an exception. Placeholder cards themselves are not sent as part of the email as they are empty. * Merge remote-tracking branch 'origin/38209-dashboard-sections' into 38209-dashboard-sections * Adding tests for placeholder cards and empty dashboards * Fix parameter mappings UI * Update section card sizes * Clean up * Add button label * Add E2E test * Revert * Reorder * Fix menu position * Assert placeholder card doesn't have a button * Fix menu tooltips * Fixing unit tests * Removed dangling code * Fixing unit tests * Remove virtual cards handling from `replaceCard` --------- Co-authored-by: Mark Bastian <markbastian@gmail.com> * bump toucan2 dependency (#38527) pulling in camsaul/toucan2#162 to fix #38516 * [QC] Remove `setDatabase` and `setDatabaseID` methods (#38550) * Remove `setDatabaseID` method from the `StructuredQuery` prototype * Remove `setDatabase` method from both Native and Structured queries * Fix time-interval against expressions (#38488) * Fix time-interval against expressions When simplifying time-interval filters, we forgot to place a temporal-unit on expressions, which meant that the date-trunc used by quarters had to match the date exactly rather than being in the interval range. * Update optimize temporal filters for expressions There is a dependency with this middleware and mongo. Without optimization mongo cannot compare the bucketed timestamps because AFAICT mongo 4 doesn't have date truncation so we do so in clojure for the compared literal but cannot do the lhs field/expression. * Avoid unnecessary re-renderings in because of getQuestions selector (#38548) * Avoid unnecessary re-renderings in DashboardSharingEmbeddingModal * Add deep equal selector * drop underscore * load analytics only when changed using a checksum (#38280) * load analytics only when changed using a checksum - over audit deserialization files * update docstrings * fix testing wording * [QC] Remove unused methods from the `StructuredQuery` prototype (#38551) * Meet embedders - minimal version (#38520) * refactor: prepare SettingsPage to have conditionally rendered steps (#38201) * refactor: prepare SettingsPage to have conditionally rendered steps * fix tests missing props * Usage reason question (#38267) * Invactive -> Inactive * add question * fix e2e test * small refactor * Update frontend/src/metabase/setup/components/UsageQuestionStep/UsageQuestionStep.tsx Co-authored-by: Mahatthana (Kelvin) Nomsawadi <me@bboykelvin.dev> --------- Co-authored-by: Mahatthana (Kelvin) Nomsawadi <me@bboykelvin.dev> * Hide "Add your data" step for people interested in embedding (#38390) * hide 'Add your data' step when user is interested in embedding * add other test cases * refactor: create selector * address pr feedback * adds minimal embed focused homepage (#38402) * very minimal homepage * fixed text color * minimal e2e + actual logic that i forgot lol * refactor the list to not have the numbers in the content * rename util functions * use button to have cursor when hovering + accessibility * refactor: move the waitFor inside the submit step * fix: make sure state is clean on multiple installs in a row * test: check that the homepage persists reload + removed typos * rename s to state * moved util functions up to home/utils.ts * adds key to avoid react warning * refactor and tests for [meet embedders] (#38465) * test: check all numbered steps during the setup flow * refactor: introduce getSteps and start using the step name instead of a number to identify steps * refactor: remove map of the steps, use step names directly * refactor: getNextStep * include 'hidden' steps in getSteps so that it won't mess up with analytics * fix after rebase issue * use full name instead of s, s -> step * analytics for meet-embedders (#38521) * refactor: extract schema version and onboarding version into constants * remove old ga events * trackUsageReasonSelected * update trackStepSeen to send the correct index * copy old schema for better diff * adds new snowplow event for usage_reason step * schema and e2e update for usage_reason_selected * adds utm tag to the link in the minimal embed homepage --------- Co-authored-by: Mahatthana (Kelvin) Nomsawadi <me@bboykelvin.dev> * Improve look of models in Browse data (#38518) Co-authored-by: Kyle Doherty <5248953+kdoh@users.noreply.github.com> Co-authored-by: Luiz Arakaki <luiz.arakaki@metabase.com> * Fix card query endpoint usage on dashboards (#38547) * Don't reload dashcards in edit mode * Update and unskip repro * Extend "duplicate tab" e2e test * Fix implicit request body schema when fetching dashcard data * Disable dashcard data refetching in edit mode * Revert `Dashboard` useEffect changes * Fix tests * Update default sizes for new dash width (#38450) * Migration adding 'width' to Dashboards 3 migrations: - 1st adding the width column with default value of 'fixed' - 2nd updating all existing dashboards to have width 'full', which corresponds to what the current behaviour is (will be the 'old' behaviour after the fixed-width project lands). - The rolloback here is necessary but we don't care what happens as the column will be dropped immediately in the next rollback anyway - 3rd sets the notNullableConstraint. DefaultNull is 'full' here, just in case there's an existing dashboard whose width value is not yet set from the 1st migration. Don't know how that could happen, but its here in case * Dashboard PUT api endpoint accepts width changes and updates appdb update-dashboard function now is aware of the :width key so those changes can end up in the transaction. Also added a width test that asserts that the value's default is "fixed", it can be changed, eg. to "full", but cannot be changed to other values. * Add width to revision tests * Fix dashboard revision tests. :width key is now needed in some revision tests. As well we need a string communicating that the :width setting has changed from 'full' to 'fixed' or vice-versa. * Fix comments/remarks in migration to be accurate * Attempt to fix default not working mysql/mariadb * Set default in dashboard model Signed-off-by: Adam James <adam.vermeer2@gmail.com> * Revert default :width 'fixed' value. * Explicitly add default value 'fixed' for MySQL/MariaDB * dashboard fixed width FE implementation * adjust popover shadow styling as the popover was blending in with the header * adjust extra button popover offset * add E2E to validate behavior * add fixed width container to public dashboards * fix public embedding not respecting dashboard width setting * add test for public dashboards * Fix embed test failures * add fixed width to x-ray dashboards * reduce code duplication * move FixedWidthContainer into DashboardGridConnected - We get the fixed-width w/o code duplication across AutomaticDashboardApp, PublicDashboard, and Dashboard * update E2E tests to reflect actual intended behavior * fix type errors * tweak default width of cards to accomodate fixed width * adjust tooltip button after merging changes from master * fix bar chart test failure * fix dashboard filters date test failure - new ellipsis button in dashboard edit mode broke the test * adjust test for new fixed width dashboards * adjust test for new fixed width dashboards * adjust test for new fixed width dashboards * bump funnel default width * adjust test for new fixed width dashboards * Fix double overlay for LineAreaBarChart dash-cards - No longer has an overlay that persists from editing mode to viewing mode - Adjust click-behavior tests to account for new grid-width. Old test relied on a dashcard being taller than they will now default to being. i.e. Move the chosen point/row to down an index. * Merge FixedWidthContainer with dashboard grid div * supress brush events while editing timeseries line charts on dashboard * fix failing test - test failed because I did not update this assertion based on previous changes * fix broken link input in editing mode * fix pointer events issue on text cards in edit mode * update tests for new dash-card default sizes * update tabs card moving test because of new default card sizes --------- Signed-off-by: Adam James <adam.vermeer2@gmail.com> Co-authored-by: Adam James <adam.vermeer2@gmail.com> Co-authored-by: adam-james <21064735+adam-james-v@users.noreply.github.com> Co-authored-by: Jesse Devaney <22608765+JesseSDevaney@users.noreply.github.com> Co-authored-by: Aleksandr Lesnenko <alxnddr@gmail.com> * add section icon and use in dashboard edit mode (#38575) * [MLv2] Use the provided `column-ref` for drills (#38047) There are cases where the ref as provided is not the same as what a fresh `(lib.ref/ref column)` would give. This seems mostly to affect models, where the metadata leaks some of the underlying details like joins. Fixes #38034. * Disable redshift support for uploads (#38583) * Disable table-privileges feature for MySQL (#38552) * Various readability improvements around based_on_upload (#38507) Co-authored-by: Callum Herries <calherries@gmail.com> * Remove flaky timeout in DelayGroup test (#38593) * Use greater than to assert timeout has elapsed * Use fake timers to test delay group * Don't instrument any namespaces for mu.fn/fn in production (#38482) * Remove handling MySQL <8 in table privileges test (#38546) * Update devcontainer Dockerile (#38598) * Embedding + required params (#38395) * fix linting after merge --------- Signed-off-by: Adam James <adam.vermeer2@gmail.com> Co-authored-by: Alexander Solovyov <alexander@solovyov.net> Co-authored-by: Anton Kulyk <kuliks.anton@gmail.com> Co-authored-by: Mark Bastian <markbastian@gmail.com> Co-authored-by: John Swanson <john.swanson@metabase.com> Co-authored-by: Nemanja Glumac <31325167+nemanjaglumac@users.noreply.github.com> Co-authored-by: Case Nelson <case@metabase.com> Co-authored-by: Uladzimir Havenchyk <125459446+uladzimirdev@users.noreply.github.com> Co-authored-by: bryan <bryan.maass@gmail.com> Co-authored-by: Mahatthana (Kelvin) Nomsawadi <me@bboykelvin.dev> Co-authored-by: Raphael Krut-Landau <raphael.kl@gmail.com> Co-authored-by: Kyle Doherty <5248953+kdoh@users.noreply.github.com> Co-authored-by: Luiz Arakaki <luiz.arakaki@metabase.com> Co-authored-by: Adam James <adam.vermeer2@gmail.com> Co-authored-by: adam-james <21064735+adam-james-v@users.noreply.github.com> Co-authored-by: Jesse Devaney <22608765+JesseSDevaney@users.noreply.github.com> Co-authored-by: Aleksandr Lesnenko <alxnddr@gmail.com> Co-authored-by: Braden Shepherdson <braden@metabase.com> Co-authored-by: Cal Herries <39073188+calherries@users.noreply.github.com> Co-authored-by: Chris Truter <crisptrutski@users.noreply.github.com> Co-authored-by: Callum Herries <calherries@gmail.com> Co-authored-by: Romeo Van Snick <romeo@romeovansnick.be> Co-authored-by: Thomas Schmidt <somtom91@gmail.com> Co-authored-by: Oleg Gromov <mail@oleggromov.com>

metabase-bot bot assigned npretto Feb 2, 2024

metabase-bot bot added the .Team/Embedding label Feb 2, 2024

npretto mentioned this pull request Feb 2, 2024

[Epic] Meet embedders at the door - minimal edition #38233

Closed

npretto force-pushed the meet-embedders-minimal-homepage branch from b374938 to 812ec45 Compare February 5, 2024 12:17

npretto commented Feb 5, 2024

View reviewed changes

npretto changed the base branch from meet-embedders-v1-ms1 to meet-embedders-hide-add-your-data February 5, 2024 16:04

npretto commented Feb 5, 2024

View reviewed changes

npretto marked this pull request as ready for review February 5, 2024 17:00

npretto requested a review from a team February 5, 2024 17:01

npretto changed the title ~~minimal homepage~~ adds minimal embed focused homepage Feb 5, 2024

Base automatically changed from meet-embedders-hide-add-your-data to meet-embedders-v1-ms1 February 6, 2024 08:27

npretto commented Feb 6, 2024

View reviewed changes

npretto added 8 commits February 7, 2024 10:47

very minimal homepage

cf33a8c

fixed text color

98b507a

minimal e2e + actual logic that i forgot lol

443ada2

refactor the list to not have the numbers in the content

f6e014a

rename util functions

e6b0289

use button to have cursor when hovering + accessibility

f78dedf

refactor: move the waitFor inside the submit step

975b5f8

fix: make sure state is clean on multiple installs in a row

176d263

npretto force-pushed the meet-embedders-minimal-homepage branch from 2c5017f to 176d263 Compare February 7, 2024 09:47

WiNloSt approved these changes Feb 7, 2024

View reviewed changes

npretto added 4 commits February 7, 2024 15:59

test: check that the homepage persists reload + removed typos

2c75c42

rename s to state

1556490

moved util functions up to home/utils.ts

e97cde2

adds key to avoid react warning

bdb803d

npretto merged commit 5fcf5e5 into meet-embedders-v1-ms1 Feb 7, 2024
105 checks passed

npretto deleted the meet-embedders-minimal-homepage branch February 7, 2024 16:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

adds minimal embed focused homepage #38402

adds minimal embed focused homepage #38402

npretto commented Feb 2, 2024 •

edited

npretto Feb 5, 2024

npretto Feb 5, 2024 •

edited

npretto Feb 5, 2024

github-actions bot commented Feb 5, 2024 •

edited

replay-io bot commented Feb 5, 2024 •

edited

npretto Feb 6, 2024

WiNloSt left a comment

WiNloSt Feb 7, 2024

npretto Feb 7, 2024

WiNloSt Feb 7, 2024

npretto Feb 7, 2024

WiNloSt Feb 7, 2024

npretto Feb 7, 2024

WiNloSt Feb 7, 2024

npretto Feb 7, 2024

	import { shouldSHowEmbedHomepage } from "../EmbedMinimalHomepage/util";
	import { shouldShowEmbedHomepage } from "../EmbedMinimalHomepage/util";

	const learnMoreUrl = useSelector(s =>
	const learnMoreUrl = useSelector(state =>

adds minimal embed focused homepage #38402

adds minimal embed focused homepage #38402

Conversation

npretto commented Feb 2, 2024 • edited

Description

How to verify

Demo

Checklist

Choose a reason for hiding this comment

npretto Feb 5, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Feb 5, 2024 • edited

replay-io bot commented Feb 5, 2024 • edited

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

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

npretto commented Feb 2, 2024 •

edited

npretto Feb 5, 2024 •

edited

github-actions bot commented Feb 5, 2024 •

edited

replay-io bot commented Feb 5, 2024 •

edited