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

Disable Error Diagnostic Modal In Embedding #42641

Merged
merged 2 commits into from
May 14, 2024
Merged

Conversation

iethree
Copy link
Contributor

@iethree iethree commented May 14, 2024

Closes #40677

Description

The error diagnostic modal should not be accessible in embedded contexts, either by keyboard, or via error components.

Embedded Non-Embedded
Screen Shot 2024-05-14 at 6 46 05 AM Screen Shot 2024-05-14 at 6 45 39 AM
Screen Shot 2024-05-14 at 6 44 05 AM Screen Shot 2024-05-14 at 6 44 35 AM

Also, the ctrl + F1 keyboard shortcut should not work when embedded.

The easiest way to test this is to modify the getIsEmbedded function to always return true;

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

@iethree iethree requested a review from a team May 14, 2024 13:04
@iethree iethree self-assigned this May 14, 2024
@metabase-bot metabase-bot bot added the .Team/AdminWebapp Admin and Webapp team label May 14, 2024
Copy link

replay-io bot commented May 14, 2024

Status Complete ↗︎
Commit 5ec60b4
Results
⚠️ 3 Flaky
2506 Passed

@iethree iethree added the no-backport Do not backport this PR to any branch label May 14, 2024

const tooltipMessage = isEmbedded
? message
: message + t` Click for more information`;
Copy link
Contributor

Choose a reason for hiding this comment

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

not sure if the leading space will get trimmed or not

@iethree iethree merged commit 5437e3d into master May 14, 2024
111 checks passed
@iethree iethree deleted the hide-error-modal-embedding branch May 14, 2024 15:36
johnswanson pushed a commit that referenced this pull request May 14, 2024
* Disable Error Diagnostic Modal In Embedding

* add e2e test for diagnostic modal
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/AdminWebapp Admin and Webapp team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Hide Error Diagnostic modal + triggers in all embedding contexts
2 participants