Embedding + required params #38395

oleggromov · 2024-02-02T14:57:54Z

Parent epic: #36524

Description

Part of the feature Let creators make parameters required, I add required parameters to the 1) question and 2) dashboard static embeddings.

How it works

When a parameter is required, it must have a default value
Embedding modal (both questions and dashboards):
- a required parameter cannot be disabled anymore ("disabled" option is disabled)
- it's value in the embedding code and JWT is no longer null by default — instead, it's set to the default value
- when a parameter is locked, the filter widget works in the special required mode, which won't allow removing the value
- when a parameter is editable, nothing changes in the modal
- in preview, filter widgets work in the special required mode too
Static embedding
- when a required parameter is locked, takes its value from the JWT
- when a required parameter is editable, renders the filter widget in the required mode
- non-required parameters should work as before
Setting up dashboard / parameter values
- when a dashboard is embedded and the parameter you're changing is disabled, we prevent it from being made required with a warning

How to check

There are a few flows we want to check. Since the logic is similar for questions and dashboards, I'll describe them together.

1. Not embedded yet question & dashboard

Create a question or a dashboard
Add required parameter(s) with default values
Observe that they can be made required as previously
Now go to the embedding settings and see that you cannot set this parameter to "disabled"
Check that when you choose "locked", you see the default values in the embedding code (JWT source)
Publish embedding and check that "editable" filters use widget in the default mode

2. Already embedded question or dashboard

Create a question or dashboard
Add parameters without making them required
In the embedding settings, make one of them "disabled" (this is the default anyway)
Edit the dashboard and observe that you cannot make this parameter required anymore

TODO

replay-io · 2024-02-02T15:11:34Z

Status	Complete ↗︎
Commit	`b9b4c81`
Results	⚠️ 3 Flaky ✅ 2279 Passed

...tend/src/metabase/public/components/EmbedModal/StaticEmbedSetupPane/StaticEmbedSetupPane.tsx

frontend/src/metabase/public/containers/PublicQuestion/PublicQuestion.jsx

frontend/src/metabase-lib/parameters/utils/parameter-values.js

WiNloSt · 2024-02-07T15:10:07Z

@oleggromov I tested the scenario we discussed on Slack.

From Conor

Making a parameter required, when it already exists and is disabled, described here
- Agree this needs to be handled to merge. Pulling the default makes sense.

Basically if we published a dashboard without required parameters. And went to edit some parameters to be required, the embedding still isn't aware of the change. Until we republish the embedding that sets the parameter to "Editable".

I kind of expect the embedding to treat disabled setting as editable as now the parameter is required.

https://www.loom.com/share/44282f926df54b3f902901c6df5d225b

WiNloSt

Looks good to me apart from the static embed that has disabled parameters and later on, has the parameter set to required.

frontend/src/metabase-lib/parameters/utils/parameter-values.js

frontend/src/metabase/public/components/EmbedModal/StaticEmbedSetupPane/ParametersSettings.tsx

frontend/src/metabase/public/containers/PublicQuestion/PublicQuestion.jsx

frontend/src/metabase/parameters/components/ParameterSettings/ParameterSettings.tsx

oleggromov · 2024-02-08T13:52:08Z

frontend/src/metabase/public/lib/types.ts

-export type EmbeddingParameters = Record<string, string>;
+export type EmbeddingParameterVisibility = "disabled" | "enabled" | "locked";
+
+export type EmbeddingParameters = Record<string, EmbeddingParameterVisibility>;


This is copied from @WiNloSt's PR https://github.com/metabase/metabase/pull/37617/files#r1482845185 and discussed with him.

oleggromov · 2024-02-08T14:40:13Z

Basically if we published a dashboard without required parameters. And went to edit some parameters to be required, the embedding still isn't aware of the change. Until we republish the embedding that sets the parameter to "Editable".

I kind of expect the embedding to treat disabled setting as editable as now the parameter is required.

https://www.loom.com/share/44282f926df54b3f902901c6df5d225b

Looks good to me apart from the static embed that has disabled parameters and later on, has the parameter set to required.

This has been addressed. Please follow this thread.

TL;DR: we don't pull defaults as it's unclear, instead we prevent a disabled (in embedding) parameter from being made required.

oleggromov · 2024-02-08T14:40:41Z

@npretto please have a look, I have addressed your feedback.

First working steps WIP update styles a bit Add required label to dashboard parameter settings pane Add getCanSaveDashboard selector and use for Save button disabled WIP Add getMissingRequiredParameters selector and use it in DashboardHeader Enable required behavior for parameter widgets on the dashboard Fix typing Fix types in tests Add required parameter to the embedding settings Support parameter defaults in embedding setup modal Update parameter widget in static embedding modal to support required Enable supported parameters for embedded dashboards Remove todo Fix parameter value in embed settings pane Support required parameters in embedded questions Remove unnecessary TODO Fixed locked parameter bug Fix tests Prevent toggling required parameter for disabled in embeds Address feedback

…s embedded and disabled

npretto

Added a small comment.

Unfortunately we found an issue:
on brand new dashboards, if you don't touch the embedding parameters, the object is empty and the logic to disable te "Required" toggle doesn't kick in.

frontend/src/metabase/parameters/components/ParameterSettings/ParameterSettings.tsx

oleggromov · 2024-02-08T18:02:17Z

on brand new dashboards, if you don't touch the embedding parameters, the object is empty and the logic to disable te "Required" toggle doesn't kick in.

This is fixed by b2cc2e8 and ad12a54

Thank you!

oleggromov · 2024-02-08T20:15:36Z

frontend/src/metabase/parameters/components/RequiredParamToggle/RequiredParamToggle.tsx

+
+  return (
+    <Flex gap="sm" mt="md">
+      <Toggle disabled={disabled} id={id} value={value} onChange={onChange} />


Right now, the disabled state in new Switch is broken. I tried to fix it but couldn't do it quickly as it requires some sorcery with the styles. So it stays the previous component for now.

already addressed

… query embed e2e tests

oleggromov · 2024-02-09T00:15:32Z

...tend/src/metabase/public/components/EmbedModal/StaticEmbedSetupPane/StaticEmbedSetupPane.tsx

+      acc[param.slug] = "enabled";
+    }
+    return acc;
+  }, defaultParams);
 }


I previously used a different but incorrect approach, which set the default value to enabled for required parameters only in the ParametersSettings component.

It was incorrect because that value was only used for rendering, and if a user published the embedding right away, without even changing any of the parameter visibility settings, the enabled won't be sent to the server. This function fixes it.

WiNloSt

Overall, this looks good to me. I've left some comments.

e2e/support/helpers/e2e-embedding-helpers.js

WiNloSt · 2024-02-09T05:09:25Z

e2e/support/helpers/e2e-embedding-helpers.js

+ * @param apiPath - "card" or "dashboard"
+ * @param callback
+ */
+export function publishChanges(apiPath, callback) {


I'm on the fence about this. We have a term called resource and resourceType

metabase/frontend/src/metabase/dashboard/components/EmbedMenu/types.ts

Line 14 in 94622e6

resourceType: "dashboard";

But resourceType can either be question or dashboard. Usually what I do is resourceType === "dashboard" ? "dashboard" : "card". That looks a bit more complicated but it keeps the same terminology.

I'll leave it to you to decide then.

I honestly don't think it's such a big issue but feel free to rename it if stays confusing :)

e2e/support/helpers/e2e-embedding-helpers.js

e2e/test/scenarios/embedding/embedding-dashboard.cy.spec.js

WiNloSt · 2024-02-09T05:31:32Z

e2e/test/scenarios/embedding/embedding-dashboard.cy.spec.js

+      // Make one parameter required
+      getDashboardFilter("Name").click();
+      toggleRequiredParameter();
+      sidebar().findByText("Default value").next().click();


I'd make the SettingValueWidget be labeled by "Default value" to make it more accessible. Now it's currently labeled by the placeholder which looks a bit wrong because the widget could show the currently selected values, but the label stays No default

If we adjust the markup we could do this in the test

Suggested change

sidebar().findByText("Default value").next().click();

sidebar().findByLabelText("Default value").click();

This is a really good suggestion, however, I'll do it outside this PR since it most likely will break other tests.

WiNloSt · 2024-02-09T05:56:16Z

e2e/test/scenarios/embedding/embedding-native.cy.spec.js

@@ -133,6 +137,59 @@ describe("scenarios > embedding > native questions", () => {
        "?id=926&created_at=&state=KS&product_id=10",
      );
    });
+
+    it("should handle required parameters", () => {
+      createAndVisitQuestion({ requiredTagName: "total", defaultValue: [100] });


nit: The name only implies we visit the question without opening the embed modal. I was confused how we could assert the embedding param without opening the embed modal, but I found out createAndVisitQuestion also opens the embed modal.

WiNloSt · 2024-02-09T06:23:53Z

frontend/src/metabase/dashboard/selectors.ts

+// Embeddings might be published without passing embedding_params to the server,
+// in which case it's an empty object. We should treat such situations with
+// caution, assuming that an absent parameter is "disabled".
+export function getEmbeddedParameterVisibility(


I'm not totally sure if we should have 2 selectors (for dashboard and question). Or have only 1 function that receives either an embedding resource or embedding parameters.

I think having only one function might be better because engineers could forget about another function when modifying one function. Both seem pretty coupled to me.

What do you think?

You're right, it would be nice to avoid it. Unfortunately, we already have lots of duplication here and elsewhere for parameters and other parts of Query Builder and Dashboards.

So since this change is consistent with the rest, I'll keep it as is. The refactoring is coming but is outside the scope of this PR.

...tend/src/metabase/public/components/EmbedModal/StaticEmbedSetupPane/StaticEmbedSetupPane.tsx

nemanjaglumac · 2024-02-09T08:40:38Z

This is a question aimed more at the product. Wouldn't it make more sense to not even show the "Disabled" option for the required parameters rather than disabling it?

uladzimirdev · 2024-02-09T09:07:39Z

frontend/src/metabase/public/components/EmbedModal/StaticEmbedSetupPane/ParametersSettings.tsx

+                  setParameterValueToDefault={() => {
+                    onChangeParameterValue(
+                      parameter.id,
+                      parameter.default as any,


why any, can it be converted to string?

It can't, parameters are often arrays of values.

okay, but can you come up with a better type than any?

I definitely will — not that I am discarding your suggestion.
But if you don't mind, as a part of a separated effort to tidy up the parameters everywhere 🙏

uladzimirdev

LGTM overall

uladzimirdev · 2024-02-09T09:09:50Z

frontend/src/metabase/parameters/components/ParameterSettings/ParameterSettings.tsx

-          </div>
-        </SettingRequiredContainer>
+        <RequiredParamToggle
+          // This forces the toggle to be a new instance when the parameter changes,


uladzimirdev · 2024-02-09T09:12:48Z

frontend/src/metabase-lib/parameters/utils/parameter-values.js

+  values = {},
+  defaultRequired = false,
+}) {
+  const value = values?.[parameter.id];


do you expect null in place of values? otherwise there is a default value {} and ?. can be removed

Yeah, you're right, it's a leftover from when it didn't have a default.

and tests failed 😄

Yeah, this is why I had it in the first place — won't touch it now though.

oleggromov · 2024-02-09T12:23:07Z

@nemanjaglumac

This is a question aimed more at the product. Wouldn't it make more sense to not even show the "Disabled" option for the required parameters rather than disabling it?

We discussed it in one of the meetings/threads and agreed that it's better to keep it disabled.
I can't find the link though.

nemanjaglumac

Others have already reviewed the code. I just want to confirm that this works as advertised. I was playing with it and tried to break it in many ways, but it's solid.

* 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 oleggromov Feb 2, 2024

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

oleggromov force-pushed the embedding-required-params branch from 9d3cd48 to 9e40c7c Compare February 5, 2024 14:05

oleggromov mentioned this pull request Feb 5, 2024

[Epic] Let creators make dashboard and native query filters required #36524

Closed

oleggromov added the no-backport Do not backport this PR to any branch label Feb 5, 2024

oleggromov force-pushed the embedding-required-params branch from 3e17d91 to e44b2a8 Compare February 5, 2024 21:24

oleggromov requested a review from a team February 6, 2024 16:25

oleggromov changed the title ~~WIP Embedding required params~~ Embedding + required params Feb 6, 2024

npretto reviewed Feb 7, 2024

View reviewed changes

...tend/src/metabase/public/components/EmbedModal/StaticEmbedSetupPane/StaticEmbedSetupPane.tsx Outdated Show resolved Hide resolved

npretto requested a review from a team February 7, 2024 12:29

npretto previously requested changes Feb 7, 2024

View reviewed changes

frontend/src/metabase/public/containers/PublicQuestion/PublicQuestion.jsx Outdated Show resolved Hide resolved

frontend/src/metabase-lib/parameters/utils/parameter-values.js Outdated Show resolved Hide resolved

npretto requested a review from a team February 7, 2024 13:08

WiNloSt reviewed Feb 7, 2024

View reviewed changes

oleggromov commented Feb 8, 2024

View reviewed changes

frontend/src/metabase/parameters/components/ParameterSettings/ParameterSettings.tsx Outdated Show resolved Hide resolved

oleggromov commented Feb 8, 2024

View reviewed changes

oleggromov force-pushed the embedding-required-params branch from 40162a1 to c6cb760 Compare February 8, 2024 14:57

oleggromov requested review from npretto, WiNloSt and a team February 8, 2024 15:00

oleggromov added 3 commits February 8, 2024 15:09

Fix tests

4f1e34e

Fix text widget default required value

7f7278f

Disable required toggle in native question parameter editor when it i…

37fed46

…s embedded and disabled

npretto reviewed Feb 8, 2024

View reviewed changes

frontend/src/metabase/parameters/components/ParameterSettings/ParameterSettings.tsx Outdated Show resolved Hide resolved

oleggromov added 2 commits February 8, 2024 17:37

Fix empty embedding_params bug in the native question parameters

b2cc2e8

Fix empty embedding_params bug in the dashboard parameters

ad12a54

Fix types and a comment

b71a4de

oleggromov requested a review from npretto February 8, 2024 18:02

oleggromov added 3 commits February 8, 2024 19:27

Update required param toggle to have a tooltip according to the desgin

bc22a6c

Extract the RequiredParamToggle component

6bb9a88

Reuse it in the question tag editor

099e817

oleggromov commented Feb 8, 2024

View reviewed changes

oleggromov added 2 commits February 8, 2024 21:20

Add index.ts

073dc78

Fix tests

450f63d

oleggromov added 3 commits February 8, 2024 21:50

Change text style

9bdb8d3

Fix the rest of the tests

6b2a9e1

Fix default embedding param settings for required params & add native…

7ee514a

… query embed e2e tests

oleggromov commented Feb 9, 2024

View reviewed changes

oleggromov added 3 commits February 9, 2024 00:16

Fix typo

7a1d715

Add embedded dashboard + required parameter e2e tests

85e7086

Dedupe tests a tiny bit

94622e6

WiNloSt approved these changes Feb 9, 2024

View reviewed changes

uladzimirdev reviewed Feb 9, 2024

View reviewed changes

oleggromov added 2 commits February 9, 2024 12:30

Address review feedback

4b416fa

Fix unit tests

b9b4c81

nemanjaglumac approved these changes Feb 9, 2024

View reviewed changes

oleggromov merged commit 140a532 into master Feb 9, 2024
107 checks passed

oleggromov deleted the embedding-required-params branch February 9, 2024 14:04

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Embedding + required params #38395

Embedding + required params #38395

oleggromov commented Feb 2, 2024 •

edited

replay-io bot commented Feb 2, 2024 •

edited

WiNloSt commented Feb 7, 2024

WiNloSt left a comment

oleggromov Feb 8, 2024 •

edited

oleggromov commented Feb 8, 2024 •

edited

oleggromov commented Feb 8, 2024

npretto left a comment

oleggromov commented Feb 8, 2024

oleggromov Feb 8, 2024

oleggromov Feb 9, 2024 •

edited

WiNloSt left a comment

WiNloSt Feb 9, 2024

oleggromov Feb 9, 2024

WiNloSt Feb 9, 2024

oleggromov Feb 9, 2024

WiNloSt Feb 9, 2024

WiNloSt Feb 9, 2024

oleggromov Feb 9, 2024

nemanjaglumac commented Feb 9, 2024

uladzimirdev Feb 9, 2024

oleggromov Feb 9, 2024

uladzimirdev Feb 9, 2024

oleggromov Feb 9, 2024

uladzimirdev left a comment

uladzimirdev Feb 9, 2024

uladzimirdev Feb 9, 2024

oleggromov Feb 9, 2024 •

edited

uladzimirdev Feb 9, 2024

oleggromov Feb 9, 2024

oleggromov commented Feb 9, 2024

nemanjaglumac left a comment

	sidebar().findByText("Default value").next().click();
	sidebar().findByLabelText("Default value").click();

Embedding + required params #38395

Embedding + required params #38395

Conversation

oleggromov commented Feb 2, 2024 • edited

Description

How it works

How to check

1. Not embedded yet question & dashboard

2. Already embedded question or dashboard

TODO

replay-io bot commented Feb 2, 2024 • edited

WiNloSt commented Feb 7, 2024

WiNloSt left a comment

Choose a reason for hiding this comment

oleggromov Feb 8, 2024 • edited

Choose a reason for hiding this comment

oleggromov commented Feb 8, 2024 • edited

oleggromov commented Feb 8, 2024

npretto left a comment

Choose a reason for hiding this comment

oleggromov commented Feb 8, 2024

Choose a reason for hiding this comment

oleggromov Feb 9, 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

nemanjaglumac commented Feb 9, 2024

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

uladzimirdev left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oleggromov Feb 9, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oleggromov commented Feb 9, 2024

nemanjaglumac left a comment

Choose a reason for hiding this comment

oleggromov commented Feb 2, 2024 •

edited

replay-io bot commented Feb 2, 2024 •

edited

oleggromov Feb 8, 2024 •

edited

oleggromov commented Feb 8, 2024 •

edited

oleggromov Feb 9, 2024 •

edited

oleggromov Feb 9, 2024 •

edited