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

chore(3000): remove notebook popover #19537

Merged
merged 10 commits into from
Jan 9, 2024
Merged

Conversation

thmsobrmlr
Copy link
Contributor

Problem

Follow up to #19535.

Changes

This PR removes the notebook popover.

I think the notebookPopoverLogic should be fully merged into notebookPanelLogic, but was a bit unsure of the changes / what the remaining code does, especially urlToAction. Let me know wether I can just move over the code or there is more to it (e.g. the logic being mounted only in certain places and that's why we listen for /*).

How did you test this code?

Re-built the app and CI run

@thmsobrmlr thmsobrmlr changed the title Remove notebook popover chore(3000Remove notebook popover Dec 28, 2023
@thmsobrmlr thmsobrmlr changed the title chore(3000Remove notebook popover chore(3000): remove notebook popover Dec 28, 2023
@thmsobrmlr thmsobrmlr mentioned this pull request Dec 28, 2023
3 tasks
@thmsobrmlr thmsobrmlr force-pushed the remove-notebook-popover branch from 9b64b83 to 9c7cd74 Compare December 28, 2023 15:24
Copy link
Contributor

github-actions bot commented Dec 28, 2023

Size Change: -2.4 kB (0%)

Total Size: 2 MB

Filename Size Change
frontend/dist/toolbar.js 2 MB -2.4 kB (0%)

compressed-size-action

@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

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

  • chromium: 0 added, 8 modified, 0 deleted (diff for shard 2)
  • webkit: 0 added, 0 modified, 0 deleted

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@thmsobrmlr thmsobrmlr requested a review from a team December 28, 2023 22:51
Copy link
Contributor

@daibhin daibhin left a comment

Choose a reason for hiding this comment

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

Looks good. Just need to remove the whole of the frontend/src/scenes/notebooks/NotebookPanel/notebookPopoverLogic.ts file

Base automatically changed from remove-sidebar to master January 9, 2024 10:52
@thmsobrmlr thmsobrmlr force-pushed the remove-notebook-popover branch from a226c5b to 0b5d881 Compare January 9, 2024 12:00
@posthog-bot
Copy link
Contributor

📸 UI snapshots have been updated

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

Triggered by this commit.

👉 Review this PR's diff of snapshots.

@PostHog PostHog deleted a comment from posthog-bot Jan 9, 2024
@thmsobrmlr thmsobrmlr enabled auto-merge (squash) January 9, 2024 12:52
@thmsobrmlr thmsobrmlr merged commit bde38d3 into master Jan 9, 2024
99 checks passed
@thmsobrmlr thmsobrmlr deleted the remove-notebook-popover branch January 9, 2024 14:22
jacobwgillespie pushed a commit to jacobwgillespie/posthog that referenced this pull request Jan 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants