Request Review #76
Request Review #76
Conversation
|
@aFuzzyBear is attempting to deploy a commit to the Pika Team on Vercel. A member of the Team first needs to authorize it. |
| @@ -9,7 +9,8 @@ Astro includes an opinionated folder layout for your project. Every Astro projec | |||
| - `public/*` - Your non-code assets (fonts, icons, etc.) | |||
| - `package.json` - A project manifest. | |||
|
|
|||
| The easiest way to set up your new project is with `npm init astro`. Check out our [Installation Guide](/quick-start) for a walkthrough of how to set up your project automatically (with `npm init astro`) or manually. | |||
| The easiest way to set up your new project is with `npm init astro`. Check out our [Installation Guide](/quick-start) for a walk-through of how to set up your project automatically (with `npm init astro`) or manually. | |||
There was a problem hiding this comment.
"walkthrough" is a double barrell word, "walk-through"
|
|
||
| `npm init astro` is the easiest way to install Astro in a new project. Run this command in your terminal to start our `create-astro` install wizard to walk you through setting up a new project. |
There was a problem hiding this comment.
your a wizard harry,
public/index.css
Outdated
| /* @media (min-width: 50em) { | ||
| :root { | ||
| --max-width: 46em; | ||
| } | ||
| } | ||
| } */ |
There was a problem hiding this comment.
Should this not be completely removed?
There was a problem hiding this comment.
aye man, however im not sure about the github etiquette for such things. So I was swithering if I should remove someone else's code or just comment it out, I thought I would go for the later, if it was approved then I would look to remove them,
There was a problem hiding this comment.
Oh not too sure, but since Git will keep history I always tend to just remove obsolete code right away.
public/index.css
Outdated
| /* font-size: 1rem; */ | ||
| /* font-size: clamp( | ||
| 0.875rem, | ||
| 0.4626rem + 1.0309vw + var(--user-font-scale), | ||
| 1.125rem | ||
| ); | ||
| ); */ |
There was a problem hiding this comment.
Here as well, complete removal right?
There was a problem hiding this comment.
Ill go and remove these then, brb
src/layouts/Main.astro
Outdated
| const btn = document.querySelector('#menu-toggle') | ||
| const sidebar = document.querySelector('#sidebar-nav') |
There was a problem hiding this comment.
Alternate option is to use getElementById('menu-toggle');
It's not a massive difference, but general one should opt for the most "clear" solution as this article states:
https://careerkarma.com/blog/javascript-queryselector-vs-getelementbyid/
There was a problem hiding this comment.
Noted, Ill have that changed man
src/layouts/Main.astro
Outdated
| btn.addEventListener('click',()=>{ | ||
| menuOpen = !menuOpen | ||
| if(menuOpen){ | ||
| sidebar.style.display="block" |
There was a problem hiding this comment.
Small lint thing, you're using double quotes here and single for none, I would opt for 1 choice
There was a problem hiding this comment.
😅 that one was a slip,
src/pages/installation.md
Outdated
| @@ -3,58 +3,67 @@ layout: ~/layouts/Main.astro | |||
| title: Installation | |||
| --- | |||
|
|
|||
| There are a few different ways to install | |||
| There are a few ways to getting started with Astro. Outlined below are instructions on how to go about installing Astro either manually or by our Astro Installer. | |||
There was a problem hiding this comment.
Would "to get started" not sound better in this sentence?
There was a problem hiding this comment.
it is, ill make that ammendment
src/pages/installation.md
Outdated
| - **A text editor** - We recommend [VS Code](https://code.visualstudio.com/) with the [Astro extension](https://marketplace.visualstudio.com/items?itemName=astro-build.astro-vscode). | ||
| - **A terminal** - Astro is mainly accessed by terminal command-line. | ||
| - **Node.js** - [`v12.20.0`](https://nodejs.org/en/download/releases/), [`v14.13.1`](https://nodejs.org/en/download/),[ `v16.0.0`](https://nodejs.org/en/download/current/), or higher. | ||
| - **A text editor** - We recommend [VS Code](https://code.visualstudio.com/) with our own [Astro extension](https://marketplace.visualstudio.com/items?itemName=astro-build.astro-vscode) for the complete DX. |
There was a problem hiding this comment.
Perhaps write out Developer Experience?
There was a problem hiding this comment.
tails it is, ill also get that amended bud
|
closing this down, to resubmit the ammended version of this PR, with the changes that @rebelchris mentioned, |
I had noticed that the button on the docs site wasnt working and there was a weird bug with the sidebar appearing as null and so the
offsetTopwasn't being applied on theSiteSidebar.astro, I then made the side more fluid, sorry to report I had to change some of the styling for the width constraints placed on the text and font sizing, otherwise everything is as it was. Applied a basic toggle to the button to make the sidebar appear. (possible resolve issue #29 )I'm going through with some copywriting for the pages atm,
If someone could review these and let me know if it was okay to do these edits, more importantly if they were done right.