Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

footer component #130

Merged
merged 5 commits into from
Dec 29, 2020
Merged

footer component #130

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f8fd139
first iteration of footer
tim-nguyen-cs Dec 24, 2020
c391211
Merge branch 'main' into feature/footer
tim-nguyen-cs Dec 25, 2020
41f3066
revised component styling
tim-nguyen-cs Dec 28, 2020
4898778
Merge branch 'main' into feature/footer
tim-nguyen-cs Dec 28, 2020
61297a9
Merge branch 'main' into feature/footer
tim-nguyen-cs Dec 29, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/views/Home/Home.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,7 @@
margin-bottom: var(--medium-spacer);
}
}

a {
color: inherit;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we use class names instead of generic attributes? I understand why we have this here, but I don't see the a tag anywhere within Home index view. Let's give the a tags a classname within the Footer component and style the attributes there.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's understandable - no problem!

58 changes: 58 additions & 0 deletions src/views/Home/components/Footer/footer.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
@use "~stylesheets/responsiveness";
@use "~stylesheets/typography";

.Footer {
max-width: 1800px;
margin: auto;
padding: var(--extra-extra-large-spacer);

@include responsiveness.respond(tab-port) {
max-width: 55%;
padding: 0;
}

@include responsiveness.respond(phone-narrow) {
max-width: unset;
}

&__images {
display: none;
justify-content: space-between;
}

&__text {
display: flex;
justify-content: space-between;

@include typography.description;
font-style: italic;

@include responsiveness.respond(tab-port) {
flex-direction: column;
text-align: center;
}
}

&__text--left {
margin: auto;
}

&__text--middle {
text-align: center;
margin: auto;
padding: 0 var(--large-spacer);

@include responsiveness.respond(tab-port) {
margin: var(--large-spacer) 0;
padding: 0;
}
}

&__text--right {
text-align: right;

@include responsiveness.respond(tab-port) {
text-align: center;
}
}
}
52 changes: 52 additions & 0 deletions src/views/Home/components/Footer/index.view.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
import React from "react";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because we upgraded to a new version of typescript in #71, we no longer have to import react if we don't use it. Instead, we can import it using the following since this is the convention that it is moving towards using this newer version of react:
import * as React from "react";

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, cool ~ no problem! In that case, we might want to change this notation for all other files too!

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah -- I believe I ran a command that changed all the existing import statements. This only appears once or twice in the codebase. I did a cmd + F on the existing codebase and it looks like I added the wrong import statement after the PR was merged in. I think we should address those in a separate PR. Thoughts?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep! I was lowkey planning a separate PR to just clean up the codebase a little bit, so that sounds like a good idea!

import "./footer.scss";

const FooterComponent: React.FC = () => {
return (
<div className="Footer">
<div className="Footer__images">
<div>Team!</div>
<div>Links!</div>
<div>Scroll!</div>
</div>
<div className="Footer__text">
<div className="Footer__text--left">
Made with ❤️☕❤️ from Santa Cruz, California.
</div>
<div className="Footer__text--middle">
CruzHacks was formerly known as HackUCSC, and launched in April 2014.
The founders were&nbsp;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SUPER NIT: I'm honestly not that bit a fan of having &nbsp; directly in the codebase as it creates a bit of messy code. We can instead style the a tag accordingly . I'd prefer if we styled it using SCSS instead of hardcoding it into the text like that.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that these entities make code look a little unclean, but I'm not quite sure what about the resolution that you mentioned. The &nbsp; entities are here, because our linter requires that content within a tags be placed on a new line. Although I personally prefer inline a tags, this causes sentences to be partitioned weirdly and requires &nbsp; used in place of any trailing spaces that are pre-maturely truncated.

Let me know if this makes sense and how to proceed - also note that this section will be removed later when this sentence is integrated into our Teams or Sponsors page.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh yes! I see what the issue is. Since the a tag has 3 props and contents within it, in order to maintain the character count for that line to 120, it makes the a go to a separate line. Adding a space at the end of the line before the a tag would cause the linter to cry since we don't want trailing spaces. In order to remove these entities, we can style it in the css by adding a margin-left: .5rem to the a tags.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ohh, that sounds like a clever fix! I'll have that done by tonight!

<a
href="https://www.linkedin.com/in/adamsmarkrichard/"
target="_blank"
rel="noreferrer"
>
Mark Adams
</a>
,&nbsp;
<a
href="https://envs.ucsc.edu/faculty/index.php?uid=bhaddad"
target="_blank"
rel="noreferrer"
>
Brent Haddad
</a>
, and&nbsp;
<a
href="https://www.linkedin.com/in/ericksondoug/"
target="_blank"
rel="noreferrer"
>
Doug Erickson
</a>
.
</div>
<div className="Footer__text--right">
© 2021 CruzHacks. All rights reserved.
</div>
</div>
</div>
);
};

export default FooterComponent;
2 changes: 2 additions & 0 deletions src/views/Home/index.view.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import * as React from "react";
import Hero from "views/Home/components/Hero/index.view";
import Footer from "views/Home/components/Footer/index.view";
ilopezro marked this conversation as resolved.
Show resolved Hide resolved
import Button from "components/Button/index.view";
import EmailSubscriptionForm from "components/EmailSubscription/index.view";
import MLHBanner from "components/MLHBanner/index.view";
Expand Down Expand Up @@ -103,6 +104,7 @@ const HomepageView: React.FC = () => {
about_text={mission_props.about}
mission_text={mission_props.mission}
/>
<Footer />
</div>
</>
);
Expand Down