Docs for how pages work #391

Merged
a3thakra merged 5 commits from adi-initial-pages-docs into main 7 months ago
Owner

#128

CI doesn't need to run/pass for this to be merged. No code changes.

#128 CI doesn't need to run/pass for this to be merged. No code changes.
a3thakra added 2 commits 8 months ago
399d0e7afd Add docs for pages
dca46d5919 Update 'docs/pages.md'
a3thakra requested review from n3parikh 8 months ago
a3thakra reviewed 8 months ago
docs/pages.md Outdated
}) as GetShapesConfig
```
Note that background shapes are not rendered into html files during build time because it is impossible to know the window dimensions. This means that you can safely use `window.innerWidth` and `window.innerHeight` as well use the width and height of the shapes container inside the `getShapesConfig` function to change the shapes based on size of the screen.
Poster
Owner

Sidenote: @a258wang do you remember why we almost never use the container's width and height in our codebase? Should this be removed?

Sidenote: @a258wang do you remember why we almost never use the container's width and height in our codebase? Should this be removed?
Owner

I'm not 100% sure why we don't use the container's width/height in our codebase... do such calculations work if JS is disabled in the browser?

I'm not 100% sure why we don't use the container's width/height in our codebase... do such calculations work if JS is disabled in the browser?
Poster
Owner

shapes are not loaded at all if JS is disabled

shapes are not loaded at all if JS is disabled
Owner

Oh I thought you were asking about why we don't use the container width/height in the other parts of the codebase... as for why the container width/height aren't used more in the shapes component, I'm not entirely sure, but I think they're used when they're needed.

Oh I thought you were asking about why we don't use the container width/height in the other parts of the codebase... as for why the container width/height aren't used more in the shapes component, I'm not entirely sure, but I think they're used when they're needed.
a3thakra marked this conversation as resolved
a3thakra requested review from a258wang 8 months ago
n3parikh requested review from j285he 8 months ago
a3thakra added 1 commit 8 months ago
cc0fd3e314 Update 'docs/pages.md'
a258wang reviewed 8 months ago
docs/pages.md Outdated
Most pages are wrapped with the [`DefaultLayout`](../components/DefaultLayout.tsx) component which limits the page width and adds the necessary margins and paddings. However, some pages need to override these default styles to accomodate for their specific design. For example:
- The [home page](../pages/index.tsx) is wider than all the other pages.
- The [about us](../pages/about/index.tsx) needs the entire screen width to properly render the bubbles.
Owner

Maybe include a link to the Bubble component, and/or provide a description of what the bubbles look like (ie. they are horizontal hot dog shaped highlights that surround some paragraphs of the text)? For someone who hasn't seen what the About Us page looks like, it might be unclear what the bubbles are.

Maybe include a link to the `Bubble` component, and/or provide a description of what the bubbles look like (ie. they are horizontal hot dog shaped highlights that surround some paragraphs of the text)? For someone who hasn't seen what the About Us page looks like, it might be unclear what the bubbles are.
a3thakra marked this conversation as resolved
j285he closed this pull request 8 months ago
j285he reopened this pull request 8 months ago
j285he approved these changes 8 months ago
docs/pages.md Outdated
All pages are a separate React component in our repository, under the [pages](../pages) folder. This is a [special directory](https://nextjs.org/docs/tag/v11.0.0/basic-features/pages) used by Next.js which maps a React component exported from this directory to a page on a url.
The React component exported by these files are wrapped by the [`App` component](../pages/_app.tsx). This lets us reuse code in between pages which makes it a good place to render the [navbar](../components/Navbar.tsx), [footer](../components/Footer.tsx), [background shapes](../components/ShapesBackground.tsx), and the general css layout of a page.
Collaborator

We could change:
component -> components (to prevent confusion in case people think it's one giant component)
css -> CSS (to be consistent with the entire readme)

Should we talk about when we used // eslint-disable-next-line @typescript-eslint/no-non-null-assertion in the dynamic pages or is that too minor?

While more details about Title component probably deserve its own docs page, should we at least mention it in pages.md since every page needs a (custom) Title?

Overall, it was very comprehensive and intuitive. LGTM!

We could change: component -> components (to prevent confusion in case people think it's one giant component) css -> CSS (to be consistent with the entire readme) Should we talk about when we used // eslint-disable-next-line @typescript-eslint/no-non-null-assertion in the dynamic pages or is that too minor? While more details about `Title` component probably deserve its own docs page, should we at least mention it in `pages.md` since every page needs a (custom) Title? Overall, it was very comprehensive and intuitive. LGTM!
a3thakra marked this conversation as resolved
n3parikh approved these changes 7 months ago
n3parikh left a comment
Owner

LGTM (with Jared/Amy's changes made)!

LGTM (with Jared/Amy's changes made)!
a3thakra added 1 commit 7 months ago
f22a931762 Upload files to 'docs/static'
a3thakra added 1 commit 7 months ago
4fcf90a8ad Address comments
a3thakra force-pushed adi-initial-pages-docs from 4fcf90a8ad to 9d33a5b834 7 months ago
Poster
Owner

No need to pass CI for this, merging.

No need to pass CI for this, merging.
a3thakra merged commit 4982b86de9 into main 7 months ago
a3thakra deleted branch adi-initial-pages-docs 7 months ago
a3thakra referenced this issue from a commit 7 months ago

Reviewers

a258wang was requested for review 8 months ago
j285he approved these changes 8 months ago
n3parikh approved these changes 7 months ago
continuous-integration/drone/push Build was killed
The pull request has been merged as 4982b86de9.
Sign in to join this conversation.
No Milestone
No project
No Assignees
4 Participants
Notifications
Due Date

No due date set.

Dependencies

No dependencies set.

Reference: www/www-new#391
Loading…
There is no content yet.