Update tech dropdown 2397

[anthonysim]

Fixes #2397

What changes did you make and why did you make them ?

• Added media query for min-width 768px. I added css grid at a width of 748px so the new dropdown would fit properly on an IPAD and IPAD Pro. The original issue did not account for other sizes except a desktop so I had to make sure everything "looked" good on an IPAD and IPAD Pro.

• Added media query for min-width 1024px. I added a width of 768px as the original issue stated.

• Ensure the dropdown matches Version A of the Projects page Figma design. The issue this PR is associated with did not state to rearrange the dropdowns as well as shrink them. TBD.

• Version C Design was implemented along with scrollbars for mobile and full screen.

• See additional notes/comments by merger @JessicaLucindaCheng: Update tech dropdown 2397 #2672 (comment)

Screenshots of Proposed Changes Of The Website (if any, please do not screen shot code changes)

Visuals before changes are applied

Visuals after changes are applied

Visuals after Version C was implemented

[github-actions]

Want to review this pull request? Take a look at this documentation for a step by step guide!

From your project repository, check out a new branch and test the changes.

git checkout -b anthonysim-update-tech-dropdown-2397 gh-pages
git pull https://github.com/anthonysim/website.git update-tech-dropdown-2397

[lopezpedres]

Availability: 10 hours
Review ETA: Wed, Jan 11, 2022

[lopezpedres]

Hi @anthonysim ! REALLY GOOD JOB. The dropdown looks really well. It ran really nice on the desktop, iPad, and iPad Pro view.
There are two main points that I just want to highlight (I am not entirely sure If they are part of this issue, we might need to create new ones)

1. According to the version A of the Figma design, the dropdown arrow should be pointing up when the dropdown is selected

   Figma Version A

   

   Antony's version

   

2. The mobile view should have a scrolling bar once the dropdown menu is active

   Figma Mobile Version

   

   Antony's version

   

@JessicaLucindaCheng please let me know what you think about these points. Otherwise, ILGTM and ready to be merged :)

[lopezpedres]

Hi @anthonysim ! REALLY GOOD JOB. The dropdown looks really well. It ran really nice on the desktop, iPad, and iPad Pro view. There are two main points that I just want to highlight (I am not entirely sure If they are part of this issue, we might need to create new ones)

1. According to the version A of the Figma design, the dropdown arrow should be pointing up when the dropdown is selected

   Figma Version A
   

   Antony's version
   

2. The mobile view should have a scrolling bar once the dropdown menu is active

   Figma Mobile Version
   

   Antony's version
   

@JessicaLucindaCheng please let me know what you think about these points. Otherwise, ILGTM and ready to be merged :)

Sorry guys! I just saw Anthony's comment about the mobile version. Please ignore the second point :) @anthonysim @JessicaLucindaCheng

[JessicaLucindaCheng]

@lopezpedres In the current website, all the dropdowns (Status, Technologies/Languages, Program Areas) have the arrows pointing down. I think the arrow of the dropdown is fine as is in terms of this issue. However, we can bring it up with the design team and ask them. We may want to ask design for an updated projects page, so all of us are on the same page in terms of what is wanted currently, including with the addition of languages from pr #2646.

[lopezpedres]

@JessicaLucindaCheng Awesome! Thanks for replying :)

[lopezpedres]

Nicely done @anthonysim !

[JessicaLucindaCheng]

Thank you for working on this issue! Your changes along with the recent inclusion of languages into the technologies dropdown look like this:

Please update your forked repo and then merge those updates into your branch. Then, please make changes so that the menu includes the languages and is still 4 columns. Thank you.

[anthonysim]

@JessicaLucindaCheng @lopezpedres, I just changed the media queries to use variables.

[anthonysim]

Thank you for working on this issue! Your changes along with the recent inclusion of languages into the technologies dropdown look like this:

Please update your forked repo and then merge those updates into your branch. Then, please make changes so that the menu includes the languages and is still 4 columns. Thank you.

So with the inclusion of the new technologies, I don't think I can follow Version A
Figma- Projects page dropdown (Version A) as the original issue (#2397) stated. I'm assuming I should do Version C now and just do the scrollbar for mobile and desktop?

[lopezpedres]

@lopezpedres I see in your screenshots that you see 4 columns for the technologies dropdown. When you were viewing it, did it include languages?

@JessicaLucindaCheng It did not. I'll attach the full screenshot:

[JessicaLucindaCheng]

@anthonysim Sorry, I don't have time to review it today. So, I will be delaying my review by one day.

• Availability: 6 hours
• ETA: Friday, January 28, 2022

[JessicaLucindaCheng]

@anthonysim The work you did to make the menus work for desktop, tablet, and mobile look fantastic. Thank you for all your hardwork!

However, I have a few more requested changes:

1. We use PR descriptions/posts to look back at the codebase commit history and see when and why changes were made. With that in mind, could you clarify the actions you took in Update tech dropdown 2397 #2672 (comment)? Also, can you just have one final picture instead of two after pictures? Also, I noticed you used em for the SCSS property left instead of px you may want to include info for your reasoning for that. Also, you may want to include your reasoning for choosing a shorter height than 493px that was recommended for the desktop dropdown in the Figma design. (I think they were good choices but I'm just wondering your reasons for doing so.)

2. For the _sass/variables/_layout.scss, I like how you are thinking for making this easier to understand but it may be confusing to include those changes with this issue because the issue has nothing to with the layouts.scss page. If you are interested, you could write up a small issue to include comments to make that page easier to read for developers and we can discuss this further if you are interested in doing so. For now, could you remove the comments you added to the layout.scss file so it was the way it was before?

If you have questions about or want to discuss any of the above, please let me know and we can find a time to meet. Again, thank you for all your hard work.

[anthonysim]

@SAUMILDHANKAR @lopezpedres @JessicaLucindaCheng, hi everyone, there is a lot of back and forth on this PR, my solution seems to not be solving the issue so I'm going to close out the PR, place the issue associated with this PR back to the backlog under projects, and unassign myself from the issue. Maybe someone else can take this and give it a try instead.

[JessicaLucindaCheng]

@anthonysim I apologize if my request for changes suggested your solution did not work because your solution does work but needed some small changes. I understand that you are frustrated since you have been working on this issue for so long. However, I think we can use all the hard work you put into your pull request so far and I would like to merge what you have so far by reopening your pr to do so. You won't have to continue work on this issue as I will assign myself to take over the issue you were working on and just continue where you left off.

[JessicaLucindaCheng]

Merging pr as is and will be fixing it in a different branch.

[JessicaLucindaCheng]

Overview

These notes are from me (@JessicaLucindaCheng), the person who merged the pr. In addition, I am taking up Anthony's issue because he has left Hack for LA and was almost done with the issue. Below are my notes about the implementation and changes made by Anthony.

Final images of Desktop version C and the mobile version implemented by Anthony Sim

Implemented Version C

Implemented mobile version with scrollbar (Note: Scrollbar not visible in the screenshot but is there)

Notes

• Differences I noticed from the Figma design

  • For the desktop version, I noticed em for the SCSS property left instead of using px for centering the tech dropdown.
  • Also, for the desktop version, the height of the dropdown is 380px for tablet and desktop views, which is a shorter height than 493px that was recommended for the desktop dropdown in the Figma design. I think this was done so that the dropdown fits in a desktop or tablet view while taking into consideration the space the fixed header navigation takes up on a screen.
  • In addition, the width of the dropdown was set to 718px instead of 768px for a tablet size screen. I think this was done because the width of a common tablet is set to 768px in _sass/variables/_layout and using the variable $bp-tablet-up in a media query for it would require the width of the dropdown to be smaller than 768px.

• Moreover, the _sass/variables/_layout.scss has added comments that are unrelated to the associated issue.