-
Notifications
You must be signed in to change notification settings - Fork 659
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: added a splashscreen lab #2106
Conversation
✅ Deploy Preview for tauri-v2 ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
cfa43ab
to
2d4e164
Compare
14d70c6
to
9986647
Compare
It's not perfect yet but it's close enough now where I'd like some input on it. 😄 |
I'm just looking at it right now lol, good timing |
what do you think of adding the full CLI as a way to quick start?
|
I'm gonna follow the lab and give my feedback based on it |
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.
well, even after the requested changed it doesn't work. Tried to fix it, broke even more. I could give another shot, or do you want to do it yourself?
Is the goal to teach about state? Because if not, I think, if possible, it should be done without it. Or else, add a requirement lab that teaches about managing state. Speaking of which, I've read many times your webpage about state and it helped me A LOT when I needed it , really!
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.
Awesome job @simonhyll, I left just a tiny suggestion 🫡
51a4063
to
e4214c9
Compare
Co-authored-by: Paul Valladares <[email protected]>
561c52c
to
07676e3
Compare
@vasfvitor The main reason I didn't do that was because it's very convenient to just add the CTA fragment we have to things and keep that as the main for everyone. Giving the full command feels less real-world adapted, the lab aims to provide some practical experience after all, might as well make people use the method they'd use the most in the future right? And if we do a full command we'd need to provide it for all package managers. |
Glad to head the article is of use to people ❤️ It's not explicitly the intent to teach about states but I also have a hard time seeing how to tackle this without states, or rather I have a hard time seeing a more appropriate solution to this specific use-case. We can definitely add a reference link to where people can read up more on states, not sure I'd say it's a prerequisite necessarily, I think I can include a very short text on what states are in the lab and it'll be good enough for the sort of copy-paste nature of labs |
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 😁
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 was going to suggest adding "decorations": false
to make it feel more splashyscreeney, but it just looks bad with square edges, I'm not sure it's possible to round it
@vasfvitor Yea it can be improved upon, especially considering right now I haven't touched upon how to do the splashscreen for mobile devices, but it's a start 🙂 We'll merge it and then get followup PR's for improving it |
What kind of changes does this PR include?
Description
Adds a new splashscreen lab.