connect settings to the embed homepage and show it #40528
Merged
Conversation
This file contains 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
npretto
changed the title
npretto
force-pushed
the
embed-homepabe-connect-settings
branch
from
9bfe50c
to
1363bb4
Compare
npretto
commented
Mar 25, 2024
| // we want to show the interactive tab for EE builds | ||
| // unless it's a starter cloud plan, which is EE build but doesn't have interactive embedding | ||
| if (isEEBuild()) { | ||
| return plan === "starter" ? "static" : "interactive"; |
There was a problem hiding this comment.
Small note: we want to show interactive even if there is no license active, the content of the card will prompt to activate the license. That's why I'm checking if it's a ee build and not if the plan is ee on line 44
npretto
commented
Mar 25, 2024
| </List.Item> | ||
| </> | ||
| <List.Item> | ||
| <Anchor |
There was a problem hiding this comment.
This change is to avoid the warning about nested tags, same for following Link > Anchors in the next files
npretto
commented
Mar 25, 2024
| @@ -42,7 +59,7 @@ export const EmbedHomepage = () => { | |||
| exampleDashboardId={exampleDashboardId} | |||
| embeddingAutoEnabled={embeddingAutoEnabled} | |||
| licenseActiveAtSetup={licenseActiveAtSetup} | |||
| plan="oss-starter" | |||
| defaultTab={defaultTab} | |||
There was a problem hiding this comment.
I changed the prop here, I think the extra abstraction wasn't worth it, the plan was only used to determine the default tab
|
Codenotify: Notifying subscribers in CODENOTIFY files for diff 34d8c79...a92764a.
|
|
npretto
force-pushed
the
embed-homepabe-connect-settings
branch
from
ad0ce08
to
0bbc409
Compare
npretto
commented
Mar 25, 2024
| @@ -57,14 +61,14 @@ export const EmbedHomepageView = (props: EmbedHomepageViewProps) => { | |||
| <Text color="text-light" size="sm"> | |||
| {/* eslint-disable-next-line no-literal-metabase-strings -- only visible to admins */} | |||
| {jt`Because you expressed interest in embedding Metabase, we took this step for you so that you can more easily try it out. You can turn it off anytime in ${( | |||
| <Link | |||
| to="admin/settings/embedding-in-other-applications" | |||
| <Anchor | |||
There was a problem hiding this comment.
Changed from Link > Anchor to <Anchor component={Link} to address warning about nested a tags
npretto
force-pushed
the
embed-homepage-settings
branch
from
437d51b
to
fc80f69
Compare
npretto
force-pushed
the
embed-homepabe-connect-settings
branch
from
0bbc409
to
dd9c7c6
Compare
npretto
force-pushed
the
embed-homepabe-connect-settings
branch
from
dd9c7c6
to
a92764a
Compare
deniskaber
pushed a commit
that referenced
this pull request
Apr 5, 2024
* create ui for the full embedding homepage, without any logic (#40251) * wip ui for the full embedding homepage * adds missing link + fix keys * remove done TODO * Update frontend/src/metabase/home/components/EmbedHomepage/InteractiveTabContent.tsx Co-authored-by: Mahatthana (Kelvin) Nomsawadi <me@bboykelvin.dev> * destructure props in the function arguments * rename customerFacingAnalyticsDocsUrl to analyticsDocsUrl --------- Co-authored-by: Mahatthana (Kelvin) Nomsawadi <me@bboykelvin.dev> * add new settings for embedding homepage (#40455) * remove old minimal version * adds new embedding homepage settings and sets them at the end of the setup * comment e2e test * add tests for embedding homepage flags * more tests * remove unit tests related to old minimal version * fixes flags descriptions * :keyword for embedding-homepage Co-authored-by: Mahatthana (Kelvin) Nomsawadi <me@bboykelvin.dev> * replace reloadSettings and refreshSettingsList with initializeSettings * `value => value === true` -> `Boolean` * toMatchObject -> toEqual for more strict tests * remove useless undefined check for settings, the component is conditionally rendered based on a setting so it means they're loaded anyway * fix: use :hidden as we changed the type to :keyword * specifically check for the `embedding` feature in the tokenFeatures object instead of checking if any feature is active * remove double \n Co-authored-by: Noah Moss <32746338+noahmoss@users.noreply.github.com> --------- Co-authored-by: Mahatthana (Kelvin) Nomsawadi <me@bboykelvin.dev> Co-authored-by: Noah Moss <32746338+noahmoss@users.noreply.github.com> * connect settings to the embed homepage and show it (#40528) * fix nested a warnings in browser console * connect settings to the embed homepage and show it * fix stories * remove localStorage.clear() as we're not using it in the logic anymore * dismiss without feedback for the embedding homepage (#40587) * feat: add dismiss functionality to the embed homepage * update setup e2e to check for the dismissal of embedding homepage * use List from metabase/ui (#40638) * set embedding-secret-key when embedding is enabled (#40813) * set embedding-secret-key when embedding is enabled * use "named" setters and getters as suggested by Noah * Fix tests for ms1 after updating on master (#41069) --------- Co-authored-by: Mahatthana (Kelvin) Nomsawadi <me@bboykelvin.dev> Co-authored-by: Noah Moss <32746338+noahmoss@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Part of [Epic] Initial homepage experience for embedding admins
This is stacked on top of add new settings for embedding homepage
This connects the settings from the previous PR to the actual homepage, and displays it.
It also contains tests for the variants/settings that are used.
The example dashboard link/button will be added in a future PR as it depends on #40066