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

Save draft config #945

Closed
wants to merge 8 commits into from
Closed

Save draft config #945

wants to merge 8 commits into from

Conversation

ptbrowne
Copy link
Collaborator

@ptbrowne ptbrowne commented Jan 24, 2023

Instead of saving the chart locally while it has not been published, we save it to the database.
This lets us share the creation link before the chart is published.
While a chart is not published, it is in draft

  • Ability to update a chart
  • Chart is auto saved when it changes
  • Share button next to publish to get either a URL bringing the person to a copy of chart, or to the chart with edit rights
  • Chart in draft should not be viewable or embeddable
  • When changing the chart title and re-opening the page, some letters are removed
  • Clean up of draft charts that have not been updated in a while

@vercel
Copy link

vercel bot commented Jan 24, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated
visualization-tool ✅ Ready (Inspect) Visit Preview 💬 1 unresolved Jan 31, 2023 at 9:11AM (UTC)

@ptbrowne ptbrowne changed the base branch from main to feat/multiple-hierarchy-roots January 24, 2023 16:23
@ptbrowne ptbrowne force-pushed the feat/multiple-hierarchy-roots branch from 3b845dc to e0d2233 Compare January 24, 2023 16:24
bprusinowski
bprusinowski previously approved these changes Jan 24, 2023
Copy link
Collaborator

@bprusinowski bprusinowski left a comment

Choose a reason for hiding this comment

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

Nice, a fantastic change 👏 💯

I wasn't able to fully test the feature, as it doesn't work on preview environment (which is expected I guess, as we don't connect to a DB there), but overall looks good!

Which option below matches copying a URL directly from the browser? Will this work or should the menu be used for sharing the chart?

Screenshot 2023-01-24 at 17 45 36

app/pages/api/config.ts Outdated Show resolved Hide resolved
app/server/nextkit.ts Outdated Show resolved Hide resolved
*/
export const updateConfig = async (
key: string,
data: Exclude<Prisma.ConfigUpdateInput["data"], undefined>,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nit: NonNullable?

app/utils/chart-config/api.ts Show resolved Hide resolved
app/configurator/configurator-state.tsx Outdated Show resolved Hide resolved
app/components/chart-selection-tabs.tsx Show resolved Hide resolved
@ptbrowne
Copy link
Collaborator Author

ptbrowne commented Jan 25, 2023

Which option below matches copying a URL directly from the browser? Will this work or should the menu be used for sharing the chart?

When copying the URL manually, you get editing rights. I think your question shows the need to expand/improve the help text like so "The other person will be able to edit your chart. This is the same URL as when you copy manually from the adress bar.". @AnninaWalker Any thoughts on this or ways to make it shorter ?

I wasn't able to fully test the feature, as it doesn't work on preview environment (which is expected I guess, as we don't connect to a DB there), but overall looks good!

I will connect the preview deployment to a DB.

@ptbrowne ptbrowne force-pushed the feat/multiple-hierarchy-roots branch 2 times, most recently from 4b005f4 to c315ea6 Compare January 27, 2023 08:05
Base automatically changed from feat/multiple-hierarchy-roots to main January 27, 2023 13:11
@ptbrowne ptbrowne changed the base branch from main to refactor/nextkit January 31, 2023 09:06
@ptbrowne ptbrowne dismissed bprusinowski’s stale review January 31, 2023 09:06

The base branch was changed.

Base automatically changed from refactor/nextkit to main February 1, 2023 15:12
@sosiology
Copy link
Contributor

this can be closed now, correct?

@ptbrowne
Copy link
Collaborator Author

Correct 👍

@ptbrowne ptbrowne closed this Feb 22, 2024
@KerstinFaye
Copy link

KerstinFaye commented Feb 23, 2024

@ptbrowne As discussed in the meeting:

One more question:

  • When I go to "preview" a draft I don't have the action to edit directly from there. Is this possible? It only allows me to copy and edit?

Improvement in the UI Design:

@ptbrowne
Copy link
Collaborator Author

Thanks Kerstin, I will implement the improvements. Quick question, I notice that you capitalize every word, is it wanted/expected ? It looks to me like a germanism 😁 I do not think personally that we should capitalize all the words like that. If there is a rule for that, I am interested.

@KerstinFaye
Copy link

KerstinFaye commented Feb 23, 2024

@ptbrowne What you are referring to is called "Title Case". It's not germanism. In UX Writing, title case is known to be better understandable. It helps differentiate your title text from your body text. It's used for call to action elements like buttons and tabs.
Have a close look on your UI of your macbook. You will find most UI elements that have less then 4 words in title case.
Screenshot 2024-02-23 at 16 48 17

@ptbrowne
Copy link
Collaborator Author

Wow, I did not know that and had never notice this in macOS 👍 Thanks for the explanation and the screenshot :)

@ptbrowne
Copy link
Collaborator Author

OK, just read a bit about that and it seems it differs from companies to companies across the web, and also depending on the country, the US being more prone to that. The rule seems to be "be consistent". Here is the article I read about this: https://uxloc.medium.com/when-to-use-sentence-case-and-title-case-161768eaab6e.

Looking at admin.ch, they do not use that much Title case. Do they have a rule about that in the official guidelines ?

@ptbrowne
Copy link
Collaborator Author

When I go to "preview" a draft I don't have the action to edit directly from there. Is this possible? It only allows me to copy and edit?

If you are the author, of the chart you should see an Edit button in the warning banner, do you see it ? Did you expect to see it a the bottom of the chart ?

image

@ptbrowne
Copy link
Collaborator Author

(I am going to apply Title Case now, but I think it is good to make sure that it is a visualize.admin guideline)

@KerstinFaye
Copy link

Is it possible to differentiate the view that a user sees when a draft url was shared with them and a preview when the chart creator clicks on preview?

@ptbrowne
Copy link
Collaborator Author

ptbrowne commented Feb 26, 2024

Yes it would be possible to differentiate the two views. I'll do that in a next PR.

@ptbrowne ptbrowne deleted the save-draft-config branch June 10, 2024 11:25
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