Skip to content
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

Dashboard sections #38463

Merged
merged 37 commits into from
Feb 8, 2024
Merged

Dashboard sections #38463

merged 37 commits into from
Feb 8, 2024

Conversation

kulyk
Copy link
Member

@kulyk kulyk commented Feb 6, 2024

Closes #38209
Fixes: #35667

Adds a concept of "sections" and "empty cards" to dashboards:

  • a placeholder card is just a blank card, but can be replaced with a question using the new "replace question" action (Add "Replace question" action to dashboard cards #36744). Technically they're implemented as virtual cards with display: "placeholder" and their own viz component (DashCardPlaceholder)
  • sections are pre-defined common layouts that might help people to focus on the dashboard content vs. layout fiddling. They use placeholder cards (although not limited to only them)

To verify

  1. Open a dashboard and enter the edit mode
  2. Click the "Add section" button at the top right
  3. Pick one of the available sections
  4. Ensure it's adding a few cards to the end of the current dashboard tab
  5. Click "Select question" on a placeholder card to replace it with a question
  6. Ensure changes get persisted (also parameters, viz setting overrides, etc.)

Note: it's expected that placeholder cards show up as blank outside the editing mode.

Demo

CleanShot.2024-02-07.at.12.55.34.mp4

@kulyk kulyk added no-backport Do not backport this PR to any branch .Team/DashViz Dashboard and Viz team labels Feb 6, 2024
@kulyk kulyk self-assigned this Feb 6, 2024
@kulyk kulyk force-pushed the ts/dashboard-card-actions-file branch 2 times, most recently from a9746cc to ed691b0 Compare February 7, 2024 10:44
@kulyk kulyk requested review from kdoh, cdeweyx and a team February 7, 2024 13:09
@kulyk kulyk marked this pull request as ready for review February 7, 2024 13:09
@kulyk kulyk requested a review from camsaul as a code owner February 7, 2024 13:09
@metabase-bot metabase-bot bot added the visual Run Percy visual testing label Feb 7, 2024
Copy link

github-actions bot commented Feb 7, 2024

Codenotify: Notifying subscribers in CODENOTIFY files for diff e7a12ce...15dd67d.

Notify File(s)
@alxnddr frontend/src/metabase/visualizations/register.js
frontend/src/metabase/visualizations/visualizations/DashCardPlaceholder.tsx

Copy link

replay-io bot commented Feb 7, 2024

Status Complete ↗︎
Commit 15dd67d
Results
⚠️ 4 Flaky
2272 Passed

@@ -168,7 +208,10 @@ export const replaceCard =
const dashboardId = getDashboardId(getState());

let dashcard = getDashCardById(getState(), dashcardId);
if (isVirtualDashCard(dashcard)) {
if (
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there cases why we need this if statement at all?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, I added it in the first place because the action wasn't meant to be used with virtual cards. But long term it would most likely just cause confusion. Removed in ea9309e

(we're still using the "replace" action only to dashcards with questions or placeholder cards)

Copy link
Member

@alxnddr alxnddr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FE code looks great, I left one minor comment.

It feels that "undo" button to remove just added section can be handy.

@alxnddr
Copy link
Member

alxnddr commented Feb 7, 2024

@markbastian just to double-check, this new card type should not affect the serialization functionality, right?

@markbastian
Copy link
Contributor

@markbastian just to double-check, this new card type should not affect the serialization functionality, right?

I do not know a lot about serialization, but I don't think it should. We might want to get a review from someone with some experience in that area to do the final PR if that's a concern.

@kulyk kulyk force-pushed the ts/dashboard-card-actions-file branch from 2ba9048 to 26c463c Compare February 7, 2024 18:08
Base automatically changed from ts/dashboard-card-actions-file to master February 7, 2024 20:28
@kulyk kulyk enabled auto-merge (squash) February 8, 2024 10:58
@kulyk kulyk merged commit 19ca99e into master Feb 8, 2024
109 checks passed
@kulyk kulyk deleted the 38209-dashboard-sections branch February 8, 2024 12:06
npretto added a commit that referenced this pull request Feb 9, 2024
* 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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no-backport Do not backport this PR to any branch .Team/DashViz Dashboard and Viz team visual Run Percy visual testing
Projects
None yet
4 participants