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: Plugin permissions component #1890

Merged
merged 19 commits into from
Aug 9, 2024

Conversation

vasfvitor
Copy link
Contributor

@vasfvitor vasfvitor commented Feb 27, 2024

This is mostly to improve DX. Right now it just gets the generated md file and outputs it. Pending i18n functionality

Preview:

But right now it is like that below, until changes are implemented upstream, or ,alternatively, process the md file in the component. I'm already checking this out.

For now this is pending tauri-plugin-workspace submodule update, but it's best to wait to update it until/if the generated markdown is changed in Tauri core tauri-apps/tauri#9019

Usage:

 <Permissions plugin="authenticator" />

Copy link

netlify bot commented Feb 27, 2024

Deploy Preview for tauri-v2 ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 064a922
🔍 Latest deploy log https://app.netlify.com/sites/tauri-v2/deploys/66b6a655ce1c5c0009a6f0b4
😎 Deploy Preview https://deploy-preview-1890--tauri-v2.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 (🟢 up 1 from production)
Accessibility: 100 (no change from production)
Best Practices: 100 (no change from production)
SEO: 92 (no change from production)
PWA: 80 (no change from production)
View the detailed breakdown and full score reports

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

@tillmann-crabnebula
Copy link
Contributor

Added the plugin identifier to the permission id in tauri-apps/tauri#10113 so you should be able to use this somehow. Also open to improve the generated tables.

@vasfvitor vasfvitor force-pushed the permissions-component branch 3 times, most recently from f60dba1 to c44291f Compare June 24, 2024 16:56
@tillmann-crabnebula
Copy link
Contributor

Hey @vasfvitor can you update the tauri and plugin workspace submodule (the plugins default permissions and the upstream PR are merged)?
Would like to see it in rendered preview :)

@tillmann-crabnebula
Copy link
Contributor

Also wondering how we want to translate the auto generated content and content based on the plugins permissions. Maybe we use some auto translation for these generated sections?

@vasfvitor
Copy link
Contributor Author

can you update the tauri and plugin workspace submodule (the plugins default permissions and the upstream PR are merged)?

@tillmann-crabnebula it looks https://github.com/tauri-apps/plugins-workspace/blob/v2/plugins/autostart/permissions/autogenerated/reference.md broken? I'll update the submodules

Also wondering how we want to translate the auto generated content and content based on the plugins permissions. Maybe we use some auto translation for these generated sections?

this is something I've been wondering since. And somehow having it auto translated would be the best, because the strings are very repetitive and prone to change. And I think it should be translated because it's part of a 'normal' page where eveything else can be translated and not a full auto generated reference page

@vasfvitor
Copy link
Contributor Author

@vasfvitor vasfvitor force-pushed the permissions-component branch 2 times, most recently from c7acded to 2be9b42 Compare July 13, 2024 14:55
@vasfvitor vasfvitor linked an issue Jul 13, 2024 that may be closed by this pull request
@vasfvitor vasfvitor force-pushed the permissions-component branch 2 times, most recently from da10890 to 7111c69 Compare July 13, 2024 20:50
@vasfvitor vasfvitor force-pushed the permissions-component branch from 7111c69 to db7e45f Compare July 23, 2024 15:31
@vasfvitor vasfvitor force-pushed the permissions-component branch from 7cb14d8 to 3643c0a Compare July 23, 2024 16:48
@vasfvitor
Copy link
Contributor Author

@simonhyll for some reason it's failing on set_statuses, any ideas?

@vasfvitor vasfvitor marked this pull request as ready for review July 23, 2024 17:10
@vasfvitor vasfvitor requested a review from a team as a code owner July 23, 2024 17:10
Copy link
Member

@dreyfus92 dreyfus92 left a comment

Choose a reason for hiding this comment

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

Really neat solution, I just left a small suggestion.

src/components/Permissions.astro Outdated Show resolved Hide resolved
vasfvitor and others added 2 commits July 23, 2024 16:33
@lucasfernog
Copy link
Member

I didn't know about this one and opened #2522 just pick the one you prefer and close the other (or we could merge this one and apply my other changes idk)

@lucasfernog
Copy link
Member

I'm kinda leaning into yours now that I have to write some scope docs.. yours is more flexible :)

@lucasfernog
Copy link
Member

Something I wish we could have is to keep the headers in the "On this page" section :/ doesn't seem like it's possible since we're inlining HTML instead of markdown

@lucasfernog
Copy link
Member

fyi alternatively we could import the markdown files directly:

import { Content as PluginPermissions } from '../../../../packages/plugins-workspace/plugins/fs/permissions/autogenerated/reference.md';

but that still doesn't preserve the headers :(

@vasfvitor
Copy link
Contributor Author

vasfvitor commented Aug 9, 2024

I didn't know about this one and opened #2522 just pick the one you prefer and close the other (or we could merge this one and apply my other changes idk)

np, I just looked at it and was thinking about using yours because you already provided more usage context. Notice that mine changes around 26 files and yours 17, so a few pages was changed without need, or are missing tables (I added to all pages with or without content)

@vasfvitor
Copy link
Contributor Author

and I always enjoy seeing how people solves the same problem that I did in a totally different way

@lucasfernog lucasfernog merged commit 8e96f89 into tauri-apps:v2 Aug 9, 2024
9 checks passed
@lucasfernog
Copy link
Member

i'll go ahead and merge this one, i wanna move a bit faster to write as much content as i can :D

lucasfernog added a commit that referenced this pull request Aug 10, 2024
known regression from #1890

the headings still aren't part of the table of contents though :(
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Does it add or improve content?
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

(next) Some of the documentation for the fs plug-in is incorrect
5 participants