www
/
www-new
12
3
Fork 5
Code Issues 58 Pull Requests 42 Projects Releases Wiki Activity

Docs for how pages work #391

Merged
a3thakra merged 5 commits from adi-initial-pages-docs into main 2022-03-10 02:47:17 -05:00
Conversation 1 Commits 5 Files Changed 2 +160
a3thakra commented 2022-01-26 22:31:43 -05:00
Owner
Copy Link

#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 requested review from n3parikh 2022-01-26 22:36:50 -05:00
a3thakra reviewed 2022-01-26 22:37:57 -05:00
docs/pages.md Outdated
@ -0,0 +68,4 @@
}) 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.
a3thakra commented 2022-01-26 22:37:57 -05:00
Author
Owner
Copy Link

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?
a258wang commented 2022-02-07 09:29:15 -05:00
Owner
Copy Link

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?
a3thakra commented 2022-02-15 00:49:24 -05:00
Author
Owner
Copy Link

shapes are not loaded at all if JS is disabled

shapes are not loaded at all if JS is disabled
a258wang commented 2022-02-15 15:12:37 -05:00
Owner
Copy Link

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 2022-01-26 22:40:26 -05:00
n3parikh requested review from j285he 2022-01-31 20:11:52 -05:00
a258wang reviewed 2022-02-07 09:32:26 -05:00
docs/pages.md Outdated
@ -0,0 +9,4 @@
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.
a258wang commented 2022-02-07 09:32:26 -05:00
Owner
Copy Link

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.
👍 1
a3thakra marked this conversation as resolved
j285he closed this pull request 2022-02-15 22:54:58 -05:00
j285he reopened this pull request 2022-02-15 22:55:03 -05:00
j285he approved these changes 2022-02-15 23:05:36 -05:00
docs/pages.md Outdated
@ -0,0 +2,4 @@
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.
j285he commented 2022-02-15 23:02:19 -05:00
Member
Copy Link

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 2022-03-09 02:14:19 -05:00
n3parikh left a comment
Owner
Copy Link

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

LGTM (with Jared/Amy's changes made)!
a3thakra force-pushed adi-initial-pages-docs from 4fcf90a8ad to 9d33a5b834 2022-03-10 02:45:46 -05:00 Compare
a3thakra commented 2022-03-10 02:46:45 -05:00
Author
Owner
Copy Link

No need to pass CI for this, merging.

No need to pass CI for this, merging.
a3thakra merged commit 4982b86de9 into main 2022-03-10 02:47:17 -05:00
a3thakra deleted branch adi-initial-pages-docs 2022-03-10 02:47:17 -05:00
Sign in to join this conversation.
Reviewers
No reviewers
a258wang
j285he
n3parikh
Labels
Clear labels   
a11y

   
Backlog

   
Blocked

   
Bug

   
Content

   
Dependencies

   
Design

   
Feature Request

   
Good First Issue

   
In Progress

   
Performance

   
Priority - High

   
Priority - Low

   
Priority - Medium

   
Untriaged
No Label
a11y
Backlog
Blocked
Bug
Content
Dependencies
Design
Feature Request
Good First Issue
In Progress
Performance
Priority - High
Priority - Low
Priority - Medium
Untriaged
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
4 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: www/www-new#391
No description provided.
Delete Branch "adi-initial-pages-docs"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?