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

Move snippets to sidepanel #15595

Merged
merged 24 commits into from
Mar 3, 2025
Merged

Conversation

adrinr
Copy link
Collaborator

@adrinr adrinr commented Feb 20, 2025

Description

Moving snippets section in bindings in order to match the UX of helpers and other bindings. We discussed keeping the existing section for crud, but this made it harder to understand and to reuse some logic. Happy to discuss improvements and different approaches.

Screenshots

Snippets will be moved to the bindings side panel

image

Snippets will not appear on the text section

Screen.Recording.2025-02-21.at.12.25.46.mov

Snippets can be added from the side panel

Screen.Recording.2025-02-21.at.12.27.19.mov

Snippets can be edited from the side panel

Screen.Recording.2025-02-21.at.12.27.44.mov

Snippets can be deleted from the side panel

Screen.Recording.2025-02-21.at.12.28.29.mov

Upgrade flow will still be in there for non-licenced users

Screen.Recording.2025-02-21.at.12.36.11.mov

Launchcontrol

Moving snippets section to the binding side panel

Feature branch env

Feature Branch Link

Copy link

linear bot commented Feb 20, 2025

Copy link

qa-wolf bot commented Feb 20, 2025

QA Wolf here! As you write new code it's important that your test coverage is keeping up.
Click here to request test coverage for this PR!

@github-actions github-actions bot added firestorm Data/Infra/Revenue Team size/l labels Feb 20, 2025
@@ -1,5 +0,0 @@
export enum SidePanel {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is only used in a file, not worth having it in types

{/if}
</Layout>
</div>

{#if showSnippetDrawer}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is done as it was the easier way to clean the state. Otherwise, when creating multiple snippets, the state would be persisted instead of starting from a clean slate everytime

Copy link
Member

Choose a reason for hiding this comment

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

Was there any way to work around this? It's not a big problem, but unmounting the component like this when hiding means we lose the drawer close animation - it just instantly disappears with a small flash rather than fading out with an animation.

Not super important but would be a nice to have I guess. Happy to keep it this way since I assume you already tried resetting state instead, but just wanted to note the downside of doing it like this.

@adrinr adrinr marked this pull request as ready for review February 21, 2025 11:36
@adrinr adrinr requested review from aptkingston and mike12345567 and removed request for aptkingston February 21, 2025 11:36
@adrinr adrinr added the feature-branch Release this PR code into a feature branch label Feb 21, 2025
@github-actions github-actions bot added the stale label Feb 28, 2025
Copy link
Member

@aptkingston aptkingston left a comment

Choose a reason for hiding this comment

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

LGTM! Just the one comment about losing the animation due to unmounting the snippet drawer, but not a big problem. All works perfectly 👍

Base automatically changed from BUDI-9068/type-sidepanel to master March 3, 2025 11:56
@github-actions github-actions bot removed the stale label Mar 3, 2025
@adrinr
Copy link
Collaborator Author

adrinr commented Mar 3, 2025

LGTM! Just the one comment about losing the animation due to unmounting the snippet drawer, but not a big problem. All works perfectly 👍

Good shout. I updated it to handle it internally

@adrinr adrinr enabled auto-merge March 3, 2025 13:13
@adrinr adrinr merged commit dcb1f3f into master Mar 3, 2025
21 checks passed
@adrinr adrinr deleted the BUDI-9068/move-snippets-to-sidepanel branch March 3, 2025 15:36
@github-actions github-actions bot locked and limited conversation to collaborators Mar 3, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
feature-branch Release this PR code into a feature branch firestorm Data/Infra/Revenue Team size/l
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants