Implemented top bar redesign #445

vladiatrinh · 2020-12-10T23:08:31Z

Summary

Redesigned the top bar by moving the course toggle from the CalendarView to the TopBar and changing the QMI Logo. For professors, I also added a toggle that switches between Student and Professor View, replacing the Switch View button in the log out menu.

Cristie's figma design for reference: https://www.figma.com/file/h0kMHuGjX9QCtZ0Y6rUbDj/Fall-2020?node-id=709%3A143

I also fixed a weird bug (which is in production QMI too), where if you click anywhere on the top bar, the modal for the log out menu appears. Now, it only appears if you click the user profile div.

Test Plan

Enroll in multiple courses in QMI where you have different roles, TA and student, for example.
Make sure that the new toggle shows up on all pages
When clicked, a menu should pop up with all the classes you are enrolled in. Your current class should have a check mark next to it.
If you aren't currently in a specific course view, the toggle should just say "Courses." This happens if you're on the Course Selection page.
If professor, make sure Prof/Student toggle switches between the two views

Notes

Breaking Changes

None

I have updated the documentation accordingly.
My PR adds a @ts-ignore

dti-github-bot · 2020-12-10T23:08:47Z

[diff-counting] Significant lines: 273.

ngwattcos · 2020-12-11T22:02:22Z

Looks very good in general! There are a couple things I want to mention here:

Interaction
The dropdown should collapse if you click on anything except for the dropdown. For example, if the dropdown for INFO 4998 is open, and you click either 1) a white space, or 2) the other dropdown, it will collapse INFO 4998.

Gradient & Width
Might be seeing things, but I believe the designs of the buttons is slightly wider, by about 50%. In addition, I believe the gradient in the design is mainly horizontal, at a slight vertical angle, around 15 degrees from vertical (used a color picker so it's just an estimation)

Design Questions

Alignment/Scroll Bars/Top Bar Hovering (absolute) Position
This probably wasn't thought up of during the design but switching from student view to professor view could cause alignment to shift when a scroll bar appears (it's a bit jarring visually but not egregious). See INFO 4998 for example.

Naming
I feel that the name Student for the student view is a bit iffy, since 1) all users can see some form of the queue and 2) students vs. tas/professors see a different view.

Prof Tags?
TAs get tags, but should professors as well?

Do you mind if we bring these questions back to design?

abcdefguan · 2020-12-16T14:22:28Z

Spotted a bug relating to this PR. Please look at issue #448 . I believe this is an old issue though (like it was an issue before this PR) but it'd be nice to fix this issue while you're at it 😄

abcdefguan · 2020-12-16T14:29:15Z

Which Queue Me In logo is correct?
Is it this one?

Or this one

I've got the feeling that

is the correct logo, but this PR appears to use

as its logo

abcdefguan · 2020-12-16T14:39:13Z

src/components/includes/TopBar.tsx

 import { Icon } from 'semantic-ui-react';
 import { logOut } from '../../firebasefunctions';
+import Logo from '../../media/Logo.svg';


import Logo from '../../media/QLogo2.svg';

abcdefguan · 2020-12-16T14:44:42Z

src/components/includes/TopBar.tsx

 }) => {
-    const history = useHistory();
-
    const [showMenu, setShowMenu] = useState(false);
    const [image, setImage] = useState(props.user ? props.user.photoUrl : '/placeholder.png');

    const userPhotoUrl = props.user ? props.user.photoUrl : '/placeholder.png';
    useEffect(() => setImage(userPhotoUrl), [userPhotoUrl]);


I'm not sure that this line of code does anything because it doesn't actually introduce any side effects (outside of React). If you think this should be removed, you should probably test whether removing it introduces any strange behaviour / regressions. Probably best to leave it alone.

abcdefguan · 2020-12-16T14:50:28Z

Looks good to me. Great Job. Some minor issues you should clear up then your PR is good to go.

ngwattcos

Hi Vladia, nice work on incorporating the design revisions!

There are still a few other things left to add (also there's a merge conflict in one file. Let me know if you need any help resolving it!):
For each dropdown, the dropdown should close as soon as you click off the dropdown (i.e., whitespace, the other dropdown, etc.)

Needs more changes.

ngwattcos

Nice work, @triple-jay! Unfortunately it seems that the Queue/Dashboard dropdown also needs to be collapsed when not clicking on the dropdown.

In addition, there is still a problem with #448 - it displays courses that have ended ((in the screenshot below, CS 3110 has already ended):

You can fix this by referring to lines 26-32 in CourseSelection.tsx, which filter courses by endDate.

ngwattcos

Looks good to me! Just be sure to rebase with the newest commits from master,

There is as major bug related to caching the last class.

ngwattcos

LGTM

vladiatrinh requested a review from a team as a code owner December 10, 2020 23:08

vladiatrinh changed the title ~~Implemented redesign for new top bar in the student and ta view~~ Implemented top bar redesign in the student and ta view Dec 10, 2020

vladiatrinh changed the title ~~Implemented top bar redesign in the student and ta view~~ Implemented top bar redesign Dec 11, 2020

abcdefguan linked an issue Dec 16, 2020 that may be closed by this pull request

Top bar does not handle courses that have ended correctly #448

Closed

abcdefguan reviewed Dec 16, 2020

View reviewed changes

ngwattcos requested changes Dec 17, 2020

View reviewed changes

ngwattcos mentioned this pull request Dec 17, 2020

Scroll Bar Should Not Resize Top Bar #453

Open

ngwattcos previously approved these changes Jan 25, 2021

View reviewed changes

ngwattcos requested changes Jan 25, 2021

View reviewed changes

ngwattcos previously approved these changes Jan 26, 2021

View reviewed changes

vladiatrinh and others added 12 commits January 26, 2021 23:23

only clicking profile triggers menu, not whole top bar

a0d9811

added new logo

641f278

added redesigned course toggle

a059b56

redesigned course menu modal

e8ad602

conditionally highlight current course in menu

d55e79d

reposition top bar

8c0d32a

fixed some styling issues

ec82174

added professor toggle

0ab9fd0

styled professor view

630ef78

changed toggle after redesign

774cb7f

changed toggle to dashboard/queue

cf3be9b

Fixed bug for clicking outside top bar

2446d81

triple-jay and others added 2 commits January 26, 2021 23:31

Minor fixes and fixed bug for spring courses

418566c

fix merge conflicts

2901b2e

ngwattcos force-pushed the vladia/new_top_bar branch from d85eeaa to 2901b2e Compare January 27, 2021 04:42

ngwattcos approved these changes Jan 27, 2021

View reviewed changes

fix linter issues

bf25afb

ngwattcos merged commit d4de532 into master Jan 27, 2021

ngwattcos deleted the vladia/new_top_bar branch January 27, 2021 04:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implemented top bar redesign #445

Implemented top bar redesign #445

vladiatrinh commented Dec 10, 2020 •

edited

Loading

dti-github-bot commented Dec 10, 2020 •

edited

Loading

ngwattcos commented Dec 11, 2020 •

edited

Loading

abcdefguan commented Dec 16, 2020 •

edited

Loading

abcdefguan commented Dec 16, 2020

abcdefguan Dec 16, 2020

abcdefguan Dec 16, 2020 •

edited

Loading

abcdefguan commented Dec 16, 2020

ngwattcos left a comment •

edited

Loading

ngwattcos left a comment

ngwattcos left a comment

ngwattcos left a comment

Implemented top bar redesign #445

Implemented top bar redesign #445

Conversation

vladiatrinh commented Dec 10, 2020 • edited Loading

Summary

Test Plan

Notes

Breaking Changes

dti-github-bot commented Dec 10, 2020 • edited Loading

ngwattcos commented Dec 11, 2020 • edited Loading

Design Questions

abcdefguan commented Dec 16, 2020 • edited Loading

abcdefguan commented Dec 16, 2020

abcdefguan Dec 16, 2020

Choose a reason for hiding this comment

abcdefguan Dec 16, 2020 • edited Loading

Choose a reason for hiding this comment

abcdefguan commented Dec 16, 2020

ngwattcos left a comment • edited Loading

Choose a reason for hiding this comment

ngwattcos left a comment

Choose a reason for hiding this comment

ngwattcos left a comment

Choose a reason for hiding this comment

ngwattcos left a comment

Choose a reason for hiding this comment

vladiatrinh commented Dec 10, 2020 •

edited

Loading

dti-github-bot commented Dec 10, 2020 •

edited

Loading

ngwattcos commented Dec 11, 2020 •

edited

Loading

abcdefguan commented Dec 16, 2020 •

edited

Loading

abcdefguan Dec 16, 2020 •

edited

Loading

ngwattcos left a comment •

edited

Loading