Extended width of years with 3 quarters to full width

[charlieweinberger]

Description

For larger screens (wider than 1648 pixels), years with 3 quarters in one row now take up the full width, instead of only taking up 3/4 of the width. Screens less wide than 1648px maintain their current functionality. Most times the breakpoint for this was 1658px, but a few times it was 1648px, and I'm not sure why it switched back and forth. I went with 1648px in the code to cover the edge case between those 10 pixels. Note that this pull request doesn't get completely rid of this problem - it just moves the breakpoint from 1648px to 1990px. This is definitely a lot better, and worth merging, but I'm not sure it's completely finished. Let me know what you think should happen at widths greater than 1990px.

Screenshots

Before:

After:

Test Plan

• Test that screen sizes less than 1648px wide, with more than 3 quarters in one year, maintain current behavior.
• Test that screens between 1648px and 1990px wide, with 3 quarters in one year, take up the full width of the year.
• Check out widths greater than 1990px and give me your thoughts on what it should look like.

Issues

Closes #559

[philosolog]

I think the resizing looks great.

Let me know what you think should happen at widths greater than 1990px.

For wider windows, I suggest that we resize the Saved Courses column to fill the empty space in the roadmap.

Here's something I quickly put together:

Edit: In my demo, I realized that the Saved Courses column is wider than it should be so I'm not sure if I should commit my changes--I'll wait for your input.

[Awesome-E]

I think I could have made this more clear, but one of the main things I wanted to tackle with #559 is that if there are 4 quarters, it should not wrap to two lines.

This does mean you'd need to adjust the class name using JS depending on how many quarters are shown. I am open to rediscussing this, but part of the objective with the previous style change was so that adding a summer quarter should not make the year be double the height. Let me know your thoughts.

Let me know what you think should happen at widths greater than 1990px.

The body wrapper should have a max width so the planner can't expand too much

[charlieweinberger]

I think I could have made this more clear, but one of the main things I wanted to tackle with #559 is that if there are 4 quarters, it should not wrap to two lines. This does mean you'd need to adjust the class name using JS depending on how many quarters are shown.

I believe I fixed the code to do this, let me know what you think.

The body wrapper should have a max width so the planner can't expand too much

When you said it "should", does that mean it already has a max width, or I need to add a max width? Because my most recent commit still doesn't look great beyond ~1900px.

[Awesome-E]

When you said it "should", does that mean it already has a max width, or I need to add a max width? Because my most recent commit still doesn't look great beyond ~1900px.

you would need to add a max width

[Awesome-E]

Given this is what I'd expect it to look like, the changes are definitely better now. What does the rest of the team think?

[charlieweinberger]

Just updated those changes that you requested. Regarding the max width, do you have any thoughts on how that should be implemented? I'm gonna try something and push some code for that soon, but if you have thoughts lmk.

[charlieweinberger]

Just pushed a potential solution for implementing a max width. The max width I chose is when the window width is 2194px and the main section of the roadmap is 1640px wide. These numbers align with the width at which a year with 6 quarters extends to exact the full width of a year.

I had to mess around with a bunch of values for the media queries to get this code to work properly. In Year.css, between lines 52 and 74, the min-width values are the values at which, if the media queries didn't exist, each year would fail to extend the full width. The minmax values within these lines are the minimum values such that the years extend the full width and only the full width (i.e. don't overflow into a 2nd row).

Note that this solution doesn't handle what happens to the course search bar after 2194px. I'm in favor of expanding its width when this breakpoint is reached, but I didn't want to change that since it would require changing a lot more code than I was comfortable doing without first checking in.

There's also some inconsistencies in my current solution which I can't figure out. When my screen (1920px wide) is minimized to 80%, a maximum width is applied to the years BEFORE years with 5 or 6 quarters can meet the full width of one row, which isn't supposed to happen (see image below). However, when I have chrome devtools open, this isn't the case - the maximum width always gets applied only after all years can extend to their max widths in one row. Let me know if you guys get this issue or not. I'm not sure if it's an issue with the max width for the year wrapper (which I've been working on), or an issue with the course search bar width (which I didn't change).

What should not be happening, but does happen when I minimize to 80% on my 1920px-wide monitor:

(notice how year 4 has 4 rows, even though there is extra space to the right of the course search bar)

What should be happening, but only happens in chrome devtools:

[charlieweinberger]

I've created the data attribute, but I can't figure out how to apply the correct widths based on that info. I'll come back to this another time, just saving my code rn.

Also, I think I messed something up when it comes to pulling/pushing/rebasing from my branch. When I tried to push, it said I need to run git pull, so I did, but then it said "warning: skipped previously applied commit [commit hash]" 8 times, then "Successfully rebased and updated refs/heads/roadmap-full-width". Is this ok?

[Awesome-E]

@charlieweinberger that should be fine. just check that when you npm run dev everything works on your branch.

[charlieweinberger]

Just pushed code that I believe fixes this issue. Let me know what you think @Awesome-E . Also note that this code uses css variables, but I can also make it work with data attributes. The method with css variables looks messier in the react file, but much nicer in the css file. The data attribute method is the opposite - it looks cleaner in the react file, but much more bloated in the css file. I prefer the css variables method.

[charlieweinberger]

@Awesome-E I present to you... infinite hard-coding.

Explanation (which is also in the code, since I think for future reference an explanation is necessary):

• Each breakpoint is the width at which a quarter can be added to a year, given a min width of 250px.
• The corresponding value of --min-quarter-width is the maximum width that a quarter can be at that breakpoint with data-max-quarter-count quarters, without overflowing into a 2nd row.
• This code works up until 2598px wide, at which quarter count 5 doesn't extend to the full width.

[Awesome-E]

I don't think it works perfectly, but there's nothing you can really do once a screen size gets too big. This is definitely good enough for what it is though – and the comment explaining the behavior of the sizing looks good to me.

[charlieweinberger]

Yea, its definitely not perfect, but at that width, some other things start to break too, like the course search floating element on the right. At that point, a more substantial change is needed, like setting a max width for the overall roadmap. That maybe be helpful, but what I have right now is definitely better than the current implementation, and most likely to not cause issues for most users. I'll merge now, and then in the future maybe we can deal with screen widths larger than 2600px.