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

fix(insights): fix insights display configs in notebooks #18456

Merged
merged 4 commits into from
Nov 7, 2023

Conversation

thmsobrmlr
Copy link
Contributor

@thmsobrmlr thmsobrmlr commented Nov 7, 2023

Problem

We decided to move forward with #18266 and #18257 to work around the issues with insight queries not updating in notebooks. (Note: both these PRs would not have been necessary with the propsQuery change mentioned further below).

The readOnly prop is preventing this to work and showHeaders: false is hiding the filters.

This still did not fix all the issues with notebooks, but removing the propsQuery did. I have no idea why, but could not find any problems in insights / dashboards / experiments. Thus suggesting we move forward with this and 🤞 everything works. If this causes any issues, we roll back the change.

Changes

This PR removes the readOnly prop, un-hides the filters and removes the propsQuery from the query selector.

How did you test this code?

Tested that editing the

@thmsobrmlr thmsobrmlr changed the title Insight display config notebooks fix(insights): fix insights display configs in notebooks Nov 7, 2023
@thmsobrmlr
Copy link
Contributor Author

@MarconLP If you see anything that breaks editing insights or insights in experiments, this PR is likely it. Please let me know if that's the case.

@thmsobrmlr thmsobrmlr enabled auto-merge (squash) November 7, 2023 17:57
@thmsobrmlr thmsobrmlr merged commit ccd53c1 into master Nov 7, 2023
71 of 72 checks passed
@thmsobrmlr thmsobrmlr deleted the insight-display-config-notebooks branch November 7, 2023 18:14
@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

2 snapshot changes in total. 0 added, 2 modified, 0 deleted:

  • chromium: 0 added, 2 modified, 0 deleted (wasn't pushed!)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

thmsobrmlr added a commit that referenced this pull request Nov 8, 2023
thmsobrmlr added a commit that referenced this pull request Nov 8, 2023
…8491)

Revert "fix(insights): fix insights display configs in notebooks (#18456)"

This reverts commit ccd53c1.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants