-
Notifications
You must be signed in to change notification settings - Fork 3
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
Save draft #1340
Save draft #1340
Conversation
ptbrowne
commented
Feb 15, 2024
•
edited
Loading
edited
- Create new chart and save as draft
- List drafts as a separate list inside the profile
- Ability to publish a draft from profile
- Ability to turn a published chart into a draft from profile
- Show the draft button inside the "configure" view
- Show the draft button inside the "layout" view
- When viewing a draft, show a warning
- When viewing a draft, ability for the owner to go to edit page
Introduction of a QueryCache mechanism similar to react-query Instead of modifying in place the data that we receive from the server, we invalidate it & refetch it
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Couldn't test yet due to failed deployment but code looks 💯 👏 Just a few remarks / questions :)
…ill take care of removing state from it
5719c8b
to
26e35ec
Compare
26e35ec
to
227c0d5
Compare
3cd66bd
to
d1ebad4
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👌
@@ -47,7 +47,7 @@ const useStyles = makeStyles<Theme>((theme) => ({ | |||
LMRPanelHeaderLayout: { | |||
width: "100%", | |||
display: "grid", | |||
gridTemplateColumns: `${DRAWER_WIDTH}px minmax(22rem, 1fr) ${DRAWER_WIDTH}px`, | |||
gridTemplateColumns: `${DRAWER_WIDTH}px minmax(22rem, 1fr) auto`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the idea here was to have a symmetrical layout, not sure if we need to have this change – if you think so, then you should also update LMRPanelHeaderLayout
class to mimic this 😄 Btw, as it's not used anywhere, maybe we can just remove this layout option :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem here was that the publish options + save draft would be wider than the panel. I think we have to see later how to improve. Myabe we can check this together tomorrow.
JSON.stringify({ | ||
state: "CONFIGURING_CHART", | ||
published_state: "PUBLISHED", | ||
...fakeVizFixture, | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe it also would make sense to apply migration here, to forget about updating this when config schema changes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will do in another PR.