Feature:-Header Redesign #366 #402

vernfongchao · 2023-01-14T22:15:10Z

Ticket: #366

What has changed:

Add Tree Icon to Header
Resize WTT LOGO
Add Navigation Buttons to Header
Add Auth Button to Header
Add Profile Image as Button to open drop down Menu
Redesign Drop down Menu
Add Icons to new drop down menu
changed color to green on burger drop down when on mobile
add logic to use burger menu vs profile image signed in vs signed out && desktop vs mobile
Add Font license to license page

What to note:

Redesign Burger Menu and Button from https://www.figma.com/file/5C3v1LUNwMPQy9JOgnKEtr/Water-the-Trees
Fullscreen logged In

mobile logged in

mobile logged out

help is grayed out, cursor set to default and doesn't redirect anywhere
New License page:

Redesign Burgur Menu and Button from https://www.figma.com/file/5C3v1LUNwMPQy9JOgnKEtr/Water-the-Trees seprate burger menu from main header file

ri0nardo · 2023-01-14T23:14:19Z

@vernfongchao what am i reviewing here?

vernfongchao · 2023-01-14T23:19:44Z

@ri0nardo sorry Ill reupload images since its a different PR, but its #366, theres some images there Ill put it on draft

zoobot · 2023-01-14T23:45:03Z

We should probably figure out a more sustainable method of importing icons than creating a new component for each icon. I would lean towards just importing them directly from mui. Are we re-using these all over the place with custom css?

vernfongchao · 2023-01-16T16:37:30Z

We should probably figure out a more sustainable method of importing icons than creating a new component for each icon. I would lean towards just importing them directly from mui. Are we re-using these all over the place with custom css?

the only custom css is just the font change, Theres a few icons that I consolidated into a single file based on the type of icon. I've always had a question about components like this where is it really useful for creating new components if there's a need of it being used again but with custom css other places vs just importing it again.

change header text to montserrat

add custom font to app.css change font header to montserrat

add props to styled/emotion button

remove commentted out code

refractor Icon imports

vernfongchao · 2023-01-19T17:32:26Z

@zoobot @mwpark2014 @ri0nardo made import change and added the font license to the bottom of the license page. please let me know if there's anything else.

mwpark2014 · 2023-01-22T18:01:26Z

The screenshots look amazing! Very excited about these changes - users will be delighted

mwpark2014 · 2023-01-22T18:20:36Z

client/src/pages/License/License.js

@@ -139,6 +139,85 @@ function License() {
          with respect to this CC0 or use of the Work.
        </div>
      </div>
+      <div>
+        <div className="license-p">
+          <h3>SIL OPEN FONT LICENSE Version 1.1</h3>


I'm not sure if we need to display this to users or if we just need it in the source code, but you would know better than me :)

I actually have no idea LOL

client/src/components/Section/Section.js

client/src/components/Header/Header.js

mwpark2014 · 2023-01-22T19:19:16Z

client/src/components/Header/Header.scss

+          &__links {
+            color: black;
+
+            &:hover {


Be careful with how heavily you nest these css selectors. This line is like 6-7 levels deep, and it seems generally accepted to keep it 3-4 levels deep (from what I can tell on https://css-tricks.com/forums/topic/sass-best-practices-nesting-more-than-3-levels-deep/).

My reasoning for not nesting too heavily:

It adds a bit more CSS that can bloat the CSS over time Ex: this CSS gets precompiled to .header __navigation __authcontainer __menu __menuItem __links __links:hover

Adds a ton of specificity to these selectors that will make it hard to maintain over time

Another resource: https://stackoverflow.com/questions/13805324/how-bad-is-it-in-practice-to-over-nest-selectors-in-sass-scss

ahh interesting, I still fairly new to scss. Ill refractor the scss to keep the nesting down.

mwpark2014 · 2023-01-22T19:27:39Z

client/src/components/Header/Menu.js

+            to="/contact"
+            className="header__navigation__authcontainer__menu__menuitem__links"
+          >
+            <MenuItem


This seems like something that can be reused. Maybe have a react component like

const YourNewComponentName = ({ children, title }) => { return <MenuItem sx={{ padding: '8px', width: '180px', }} onClick={handleClose} > {children} <span className="header__navigation__authcontainer__menu__menuitem__links__span"> {title} </span> </MenuItem> }

good idea actually, I had trouble with the Menu not anchoring correctly to the button, but the menu items shouldnt be an issue will look into refractoring that

mwpark2014 · 2023-01-22T19:31:32Z

client/src/components/Header/Header.scss

@@ -4,15 +4,153 @@
    url('../../assets/fonts/WTT-THIN.ttf') format('truetype');
 }

+// .header {


I think SASS works for this. But IIRC, you tried styled components first. IF you do want to implement this with styled components, I think this reference would help write styled components in a more concise and reusable way: https://css-tricks.com/dry-ing-up-styled-components/

Basically, styled components are designed to be reusable and modular by doing some good old-fashioned inheritance

I'm open to having this discussion. Its just hard for me as I don't really know what conventions there are for styled components especially when it comes to using it for page structuring and being imported and being reused constantly

Okay, we can have that discussion on styled components vs SCSS on Wednesday. If we do go with styled components, we can go through some examples and establish a convention for the project

Yeah sounds like a plan!

Add Icons for Header

d0ea6ac

Redesign Burgur Menu and Button from https://www.figma.com/file/5C3v1LUNwMPQy9JOgnKEtr/Water-the-Trees seprate burger menu from main header file

vernfongchao requested review from zoobot and ri0nardo January 14, 2023 22:16

commented out font change for now

3bc5112

vernfongchao marked this pull request as draft January 14, 2023 23:19

vernfongchao added 3 commits January 16, 2023 11:38

add montserrat font

4fd9823

change header text to montserrat

remove extra montserrat font but regular

ed2c1d0

add custom font to app.css change font header to montserrat

remove useStates on hover effect

6457cea

add props to styled/emotion button

vernfongchao changed the title ~~vernfongchao/feature/Header/Redesign~~ Feature: Header Redesign #366 Jan 17, 2023

vernfongchao changed the title ~~Feature: Header Redesign #366~~ Feature:-Header Redesign #366 Jan 17, 2023

vernfongchao marked this pull request as ready for review January 17, 2023 17:43

vernfongchao added 6 commits January 17, 2023 13:06

fix BEM styling

41e5f81

remove commentted out code

Merge branch 'main' into vernfongchao/feature/header/redesign

c796c36

merge with main

e28fbea

remove unused imports

b65db70

remove font license text and add to license page

d0beadd

refractor Icon imports

remove font license file

e8e24c6

vernfongchao requested a review from mwpark2014 January 19, 2023 17:31

fix type and naming convention on license page

4bdbc82

mwpark2014 requested changes Jan 22, 2023

View reviewed changes

vernfongchao marked this pull request as draft January 22, 2023 20:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature:-Header Redesign #366 #402

Feature:-Header Redesign #366 #402

vernfongchao commented Jan 14, 2023 •

edited

Loading

ri0nardo commented Jan 14, 2023

vernfongchao commented Jan 14, 2023

zoobot commented Jan 14, 2023

vernfongchao commented Jan 16, 2023

vernfongchao commented Jan 19, 2023 •

edited

Loading

mwpark2014 commented Jan 22, 2023

mwpark2014 Jan 22, 2023

vernfongchao Jan 22, 2023

mwpark2014 Jan 22, 2023

vernfongchao Jan 22, 2023

mwpark2014 Jan 22, 2023

vernfongchao Jan 22, 2023

mwpark2014 Jan 22, 2023

vernfongchao Jan 22, 2023

mwpark2014 Jan 24, 2023

vernfongchao Jan 25, 2023

Feature:-Header Redesign #366 #402

Are you sure you want to change the base?

Feature:-Header Redesign #366 #402

Conversation

vernfongchao commented Jan 14, 2023 • edited Loading

Ticket: #366

What has changed:

What to note:

ri0nardo commented Jan 14, 2023

vernfongchao commented Jan 14, 2023

zoobot commented Jan 14, 2023

vernfongchao commented Jan 16, 2023

vernfongchao commented Jan 19, 2023 • edited Loading

mwpark2014 commented Jan 22, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vernfongchao commented Jan 14, 2023 •

edited

Loading

vernfongchao commented Jan 19, 2023 •

edited

Loading