Get Involved Page! #76

Merged
dora merged 14 commits from feat/get-involved-page into main 2 years ago
dora commented 2 years ago (Migrated from git.uwaterloo.ca)
Collaborator

closes #13 and #24

  • added the <EmailSignup> and <ConnectWithUs> components!
  • adds the Get Involved Page
closes #13 and #24 - added the `<EmailSignup>` and `<ConnectWithUs>` components! - adds the Get Involved Page
d43su added 1 commit 2 years ago
d477989fc3 Merge branch 'main' into feat/get-involved-page
d43su added 3 commits 2 years ago
d43su added 1 commit 2 years ago
f589b05fb8 mobile styles and images
Collaborator

Two different codeys (mobile vs desktop) is most likely a mistake. Confirm with design about this.

Two different codeys (mobile vs desktop) is most likely a mistake. Confirm with design about this.
d43su added 1 commit 2 years ago
48abedd17d new desktop codey image
a3thakra reviewed 2 years ago
}
.socialLinks {
margin: calc(36rem / 16) calc(-10rem / 16);
Collaborator

Change the CSS of the Social links component instead. Make sure that the first child of SocialLinks has margin-left: 0 and last child of SocialLinks has margin-right: 0

Change the CSS of the Social links component instead. Make sure that the first child of SocialLinks has margin-left: 0 and last child of SocialLinks has margin-right: 0
a3thakra marked this conversation as resolved
a3thakra reviewed 2 years ago
export function ConnectWithUs() {
return (
<div>
Collaborator

<section> tag instead? 🤔 What do you think?

`<section>` tag instead? :thinking: What do you think?
a3thakra marked this conversation as resolved
a3thakra reviewed 2 years ago
}
.socialLinks > * {
justify-content: left;
Collaborator

same here, change the css in the social links component so it does not center all the links.

same here, change the css in the social links component so it does not center all the links.
a3thakra marked this conversation as resolved
a3thakra reviewed 2 years ago
@media only screen and (max-width: calc(768rem / 16)) {
.header {
font-size: calc(24rem / 16);
line-height: calc(36rem / 16);
Collaborator

This is wrong for two reasons:

  • line-height should be a unitless value.
  • if you are using a unitless value, then you should not be dividing by 16, but should be dividing by what the font-size is (36 / 24)

In this case, line-height would become 1.5, which is already the case for all headings. (_app.css) So you shouldn't need to set it here.

This is wrong for two reasons: - line-height should be a **unitless** value. - if you are using a unitless value, then you should not be dividing by 16, but should be dividing by what the font-size is (36 / 24) In this case, line-height would become 1.5, which is already the case for all headings. (_app.css) So you shouldn't need to set it here.
a3thakra marked this conversation as resolved
a3thakra reviewed 2 years ago
@media only screen and (max-width: calc(768rem / 16)) {
.container > * {
margin: 0 0 calc(15rem / 16) 0;
Collaborator

might as well make it 1 rem lol

might as well make it 1 rem lol
a3thakra marked this conversation as resolved
a3thakra reviewed 2 years ago
export function EmailSignup() {
return (
<div className={styles.container}>
Collaborator

<section> tag here too?

`<section>` tag here too?
a3thakra reviewed 2 years ago
<form className={styles.form} action="">
<Input type="text" placeholder="Full Name*" required />
<Input type="email" placeholder="Email*" required />
<Button className={styles.button}>Subscribe</Button>
Collaborator

use <Button type="submit" so that people can press enter inside an input to subscribe.

use `<Button type="submit"` so that people can press enter inside an `input` to subscribe.
a3thakra marked this conversation as resolved
a3thakra reviewed 2 years ago
}
.container > * {
margin: 0 0 calc(21rem / 16) 0;
Collaborator

add a padding to the container instead

add a padding to the container instead
a3thakra marked this conversation as resolved
a3thakra reviewed 2 years ago
display: block;
width: 100%;
margin: calc(21rem / 16) 0;
padding: calc(10rem / 16) calc(31rem / 16);
Collaborator

can we do padding: calc(10rem / 16) calc(32rem / 16); pls 🙈

can we do `padding: calc(10rem / 16) calc(32rem / 16);` pls 🙈
a3thakra marked this conversation as resolved
a3thakra reviewed 2 years ago
## How to Join
Collaborator

Make sure that the content matches the content on notion:

https://www.notion.so/Get-Involved-DONE-4dd998c0c13d428894961796942fcaef

Make sure that the content matches the content on notion: https://www.notion.so/Get-Involved-DONE-4dd998c0c13d428894961796942fcaef
a3thakra marked this conversation as resolved
a3thakra added 1 commit 2 years ago
f2ca41f6d7 Address PR feedback
a3thakra reviewed 2 years ago
import styles from "./get-involved.module.css";
/**
* @todo fix codey image with svg
Collaborator

? is it not fixed?

? is it not fixed?
a3thakra reviewed 2 years ago
margin-top: calc(50rem / 16);
}
/* @todo aaaaaa */
Collaborator

is it still not fixed?

is it still not fixed?
a3thakra added 1 commit 2 years ago
499e7865ba Remove todos
a3thakra merged commit 360d9e4b6f into main 2 years ago
a3thakra referenced this issue from a commit 2 years ago
a3thakra deleted branch feat/get-involved-page 2 years ago
continuous-integration/drone/push Build is passing
The pull request has been merged as 360d9e4b6f.
Sign in to join this conversation.
No reviewers
No Milestone
No project
No Assignees
2 Participants
Notifications
Due Date

No due date set.

Dependencies

No dependencies set.

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