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

feat: create JS and Rust dialog guide #1623

Merged
merged 22 commits into from
Nov 30, 2023
Merged

Conversation

vasfvitor
Copy link
Contributor

@vasfvitor vasfvitor commented Oct 16, 2023

What kind of changes does this PR include?

  • Minor content fixes (broken links, typos, etc.)
  • New or updated content

Description

  • Partially Addresses Create Dialog guide #1477
    -I'm still unsure what we should include or exclude from the guide. I mean, there are more options to each function that I didn't specify but it's written in the JS api docs.
  • Fixed typo on notification guide.
    - This only includes JavaScript

@netlify
Copy link

netlify bot commented Oct 16, 2023

Deploy Preview for tauri-docs-starlight ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 014a5da
🔍 Latest deploy log https://app.netlify.com/sites/tauri-docs-starlight/deploys/6568b17601f5da000870e853
😎 Deploy Preview https://deploy-preview-1623--tauri-docs-starlight.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
1 paths audited
Performance: 100 (no change from production)
Accessibility: 100 (no change from production)
Best Practices: 100 (no change from production)
SEO: 92 (no change from production)
PWA: -
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify site configuration.

@lorenzolewis
Copy link
Member

Thanks for this! Overall really like the simplicity of this. For the headings could we use them to describe what the function is doing more clearly? For example: instead of "Ask" as a heading, we could instead have "Create Yes/No Dialog". This way the reader is able to quickly look at the TOC and jump to the desired result they'd like.

@vasfvitor
Copy link
Contributor Author

Ok. Done

Copy link
Member

@lorenzolewis lorenzolewis left a comment

Choose a reason for hiding this comment

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

Awesome! Just a few adjustments. Would you like to take a stab at including Rust in this as well or would you like to leave that as a follow-up PR for you or someone else?

src/content/docs/features/dialog.mdx Outdated Show resolved Hide resolved
```js
// Open a dialog
const file = await open({
multiple: false,
Copy link
Member

Choose a reason for hiding this comment

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

Instead of listing out each permutation of multiple and directory I think just adding a paragraph before the code block explaining what multiple and directory mean would be just as effective and reduce a lot of duplication.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I reduced the duplication. Now, since there are multiple options, should those be explained as well? Not only the Open dialog, but Save, Message, and Confirm function, each have multiple options.

Current Open dialog (The link to js api is inside the JavaScript tab)

To compare with Save dialog

Copy link
Member

Choose a reason for hiding this comment

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

I think how you've done in the "Open" one is perfect by linking to the API references. This lets the reader understand the very basic functionality (which is what this page is for) and let's them jump off to customise and configure it from there (which is what the reference page is for).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I separated Rust and JavaScript guides into 2 blocks and moved stuff around. I hope that is ok. My reasoning is that this plugin have different ways to be used that does not translate into 1:1 blocks.

Also moved the link of Rust docs and JavaScript docs to the start of the block. Since it should be basic functionality I guess, content-wise, ready.

@vasfvitor
Copy link
Contributor Author

I'll write the Rust guide

@vasfvitor vasfvitor changed the title feat: create JS dialog guide feat: create JS and Rust dialog guide Nov 10, 2023
@vasfvitor vasfvitor marked this pull request as draft November 10, 2023 18:37
@vasfvitor vasfvitor marked this pull request as ready for review November 29, 2023 23:55
@lorenzolewis
Copy link
Member

I'm good with splitting them out when it's not 1:1. I would just increase all of the headings under "Usage" by one so that the JS and Rust headings (and their specific children) sit nicely under "Usage". You could also add i18nReady: true to the frontmatter now that this is written to signal that it's available for translators.

@vasfvitor
Copy link
Contributor Author

Done and done. Way better with the headings this way

@lorenzolewis lorenzolewis merged commit a4ac504 into tauri-apps:next Nov 30, 2023
6 checks passed
@vasfvitor vasfvitor deleted the guides branch February 25, 2024 21:53
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.

2 participants