Bret/update quickstart line marks #2720
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
💡 Enable Vercel Agent with $100 free credit for automated AI reviews |
| - [`<SignInButton />`](/docs/reference/components/unstyled/sign-in-button): An unstyled component that links to the sign-in page. In this example, since no props or [environment variables](/docs/guides/development/clerk-environment-variables) are set for the sign-in URL, this component links to the [Account Portal sign-in page](/docs/guides/customizing-clerk/account-portal#sign-in). | ||
|
|
||
| ```astro {{ filename: 'src/layouts/Layout.astro' }} | ||
| ```astro {{ filename: 'src/layouts/Layout.astro', mark: [[1, 5], [17, 27]] }} |
There was a problem hiding this comment.
This is the only one I question. Feels like lines 2 and 20-25 is the more specific Clerk parts to highlight.
There was a problem hiding this comment.
I'm split – I highlighted all the lines that weren't in the generated Layout.astro file
Perhaps we remove title? This is also the only guide that is using <nav> inside of <header>, so we could remove that as well to stay in parody with the other guides.
There was a problem hiding this comment.
Here's the updated version – I also folded the CSS
There was a problem hiding this comment.
Why add <header /> into the highlights?
On second look, I see you're doing the in the React quick starts too.
There was a problem hiding this comment.
Looking at other SDKs, we include the header as part of the highlighting, so I'd follow that for consistency on both astro and react changes, it seems.
There was a problem hiding this comment.
I am on board with whatever is consistent.
There was a problem hiding this comment.
While editing these, I followed the steps to create the project using npx create-[...]-app/etc, and then looked at what the line differences were. Any lines that we added to the file, I highlighted, as I had noticed that was the pattern we were doing across other quickstart guides.
| <a href="https://vite.dev" target="_blank"> | ||
| <img src={viteLogo} className="logo" alt="Vite logo" /> | ||
| </a> | ||
| <a href="https://react.dev" target="_blank"> | ||
| <img src={reactLogo} className="logo react" alt="React logo" /> | ||
| </a> | ||
| </div> | ||
| <h1>Vite + React</h1> | ||
| <div className="card"> | ||
| <button onClick={() => setCount((count) => count + 1)}>count is {count}</button> | ||
| <p> | ||
| Edit <code>src/App.tsx</code> and save to test HMR | ||
| </p> | ||
| </div> | ||
| <p className="read-the-docs">Click on the Vite and React logos to learn more</p> | ||
| </> |
There was a problem hiding this comment.
Why add all of this tho? Think we want to keep the snippet as straightforward as possible.
There was a problem hiding this comment.
Answered in the below comments because I think I'm making certain assumptions that I shouldn't 😅
| import reactLogo from './assets/react.svg' | ||
| import viteLogo from '/vite.svg' | ||
| import './App.css' |
There was a problem hiding this comment.
Same comment with these imports.
There was a problem hiding this comment.
Why add all of this tho? Think we want to keep the snippet as straightforward as possible.
I agree that keeping snippets straightforward is ideal. I've also noticed that many of the quickstart guides include the rest of the generated file from the `npx create-[...]-app" results in the snippet, and then fold them, as I've done here.
Where exactly should I draw the line between "make sure this snippet is representative of the generated file" and "remove anything irrelevant from the generated file"?
For example, is persisting the template's React & Vite logos necessary?
There was a problem hiding this comment.
That's fair. I think this one is particularly flagrant to me because of the count code that is irrelevant to the rest of the code compared to other setup code in other SDKs. IMO, keeping the imports at the start would be fine, but I don't think we need all the code related to the count stuff.
A user experience is usually to copy the snippet and paste that into the file, which then removes the initial code anyway. So to me, that makes sense. I'd be keen to hear @alexisintech thoughts on this actually.
There was a problem hiding this comment.
i think adding syntax highlighting is fine, but let's not add anything else to the quickstart
| import './App.css' | ||
|
|
||
| function App() { | ||
| const [count, setCount] = useState(0) |
There was a problem hiding this comment.
[cont] Is App.css something we should remove?
4 participants
🔎 Previews:
Astro Quickstart
Before:
After:
Expo Quickstart
Before:
After:
React Quickstart
Marks
Before:
After:
Full `pages/_app.tsx` file
Before:
After:
Next.js Pages Quickstart
Before:
After:
What does this solve?
What changed?
Checklist