699 Update responsive design up to 200% zoom

[Kenny4297]

Description

I have implemented media queries across all CSS files pertaining to the Header. These modifications ensure proper display and functionality of the header at up to 200% zoom.

Screenshots

Here's how the header looks at 200% zoom:

Notes

While this project's structure—with multiple components and corresponding CSS files—works well with JavaScript logic implementation, it presents challenges in styling, particularly for the header.

Concerns

In my personal opinion, I would be worried about the scalability of this project when it comes to responsive design. Although it is possible to make this application responsive, the fact that multiple CSS files are used (and scattered) throughout the application with regards to one section of the application (the header, for example) will make responsive design difficult.

Thanks for letting me contribute!!

[pmarsh-scottlogic]

Hiya @Kenny4297 - that was fast work!

Regarding the concerns - yes, I agree it is hard to make the site responsive! Have you come across any project structures that better enable responsive design? Any examples? We're happy to learn!

Make sure you run the formatter on the frontend - the workflow is complaining. (cd frontend then npm run format)

And finally a heads up and apology - all the maintainers work on this as part of our day job, and it is coming up to Christmas hols, so I'm afraid we'll only be able to give this PR some attention after the new year. So take care until then :)

p.s. for my own interest, how did you come across the project?

[Kenny4297]

Hi Scott: Regarding project structure: From my experiences, I would put all the header CSS in one file. This way it makes things much easier. Formatter: I can run the formatter and give you another PR soon. How I found this project: I used the following site: https://goodfirstissues.com/index.html It's surprising how difficult many of the first issues are, so to keep things simple (especially with my day job as a lead engineer) I use this site to quickly find any applications that need 'basic' assistance. Additional Notes: As an AI engineer myself, I am quite keen on the type of code that AI puts out. In my personal opinion, some of the code is over complicated and can be refactored. The much of the code in the header, in my opinion, is overkill, and thus it makes it hard to edit and adjust. Thanks again for letting me work on this, it was a challenge! Also, connect with me on LinkedIn! https://www.linkedin.com/in/kedgard-cordero/ Happy Holidays, Kedgard

…

On Fri, Dec 22, 2023 at 11:54 AM Peter Marsh ***@***.***> wrote: Hiya @Kenny4297 <https://github.com/Kenny4297> - that was fast work! Regarding the concerns - yes, I agree it is hard to make the site responsive! Have you come across any project structures that better enable responsive design? Any examples? We're happy to learn! Make sure you run the formatter on the frontend - the workflow is complaining. (cd frontend then npm run format) And finally a heads up and apology - all the maintainers work on this as part of our day job, and it is coming up to Christmas hols, so I'm afraid we'll only be able to give this PR some attention after the new year. So take care until then :) p.s. for my own interest, how did you come across the project? — Reply to this email directly, view it on GitHub <#727 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AXE6WKRMEXYMKBORDML43V3YKXCL7AVCNFSM6AAAAABBABWS3CVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNRXHE2DAMRUGE> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[pmarsh-scottlogic]

I think it's a great idea to incorporate media queries to help with different sizes. I have a few main comments to make to start with.

I would prefer to not change so much of the font sizing in the media queries. Since all the font sizes are defined in rem already, it means that when you adjust the zoom level, all the fonts will scale exactly according to that zoom level.

I notice that, on my laptop screen, the header fonts appear smaller at zoom levels 150% and 200% when compared to 100%! We'd really like to avoid this since the idea behind us supporting zoom levels is mainly to support users who struggle with sight. We'd much prefer to adjust the layout, and padding to make room rather than adjusting fonts. See the screenshot on the issue (labelled "Roughly expected outcome") :)

I wonder if, to support our current css structure, we could put the media queries within our classes? So instead of

@media (...) {
  .my-class {...}
  .my-other-class{...}
}

we do
```css
.my-class {
  @media(...){...}
}

.my-other-class {
  @media(...){...}
}

These are my first thoughts as I look at the PR. It might be worth waiting a couple of weeks for the more senior folk on my team to respond because they might disagree with me, or have more concrete suggestions.

Again, sorry that the PR isn't getting so much attention at the minute! In the next week or two, we'll get back to our regular schedules and have time to look at it.

[pmarsh-scottlogic]

I like the idea of hiding icons to make space

[pmarsh-scottlogic]

can be shortened

Suggested change

	padding: 0rem 0rem;
	padding: 0;

[pmarsh-scottlogic]

can be shortened to

Suggested change

	gap: 0rem;
	gap: 0;

[pmarsh-scottlogic]

Suggested change

	padding: 0.4rem 0.4rem;
	padding: 0.4rem;

[Kenny4297]

I really like the changes that have been implemented. I'd like to provide some additional insights:

1) Scaling of Font Sizes with Zoom Levels

Observation:

"Since all the font sizes are defined in rem, it means that when you adjust the zoom level, all the fonts will scale exactly according to that zoom level."

Comment:

• This statement may not be entirely accurate. rem units are relative to the root element's font size, not directly responsive to changes in the window size like percentages or viewport units (vw, vh).
• To make font sizes responsive to window size using rem, you would typically adjust the root element's font size with media queries.

Suggestion:

• What you could do is set the root font sizes through media queries. This way, font sizes defined in rem within those roots would change automatically in response to window size adjustments.

2) Shortening '0rem' to '0'

Proposal:

• Add all these modifications to your formatter!. This could streamline the process and ensure consistency across the codebase.

Note:

• Due to my current workload, I might not be able to revisit this for the next few months. I'm grateful for the opportunity to contribute and look forward to seeing how this project progresses.

Thanks for letting me work on this!

Best regards,
Kedgard

[gsproston-scottlogic]

Hi Kedgard,

Thanks for your work on this, and your patience while most of us were away over the winter break.
If you're unable to work on this for the next few months, I'll close this PR if that's all right.
Again, thank you for your work and your insight regarding the project structure and adding formatting rules for CSS, we'll definitely be looking into those!

Kind regards,
George

[chriswilty]

@Kenny4297 FWIW We've not really looked into responsive design yet, and we don't even have issues in our backlog for it, at the moment, as there are UX challenges around using this application on smaller devices. That being said, as you suggest we would likely want to use a limited set of media queries to set some values application-wide, such as base font size and maybe a spacing/padding strategy.

Zoom is of course tricky, because it can affect which media query is active. We could maybe use resolution in dppx to distinguish if we are zoomed in or just on a smaller device. As @pmarsh-scottlogic points out, we don't really want an increase in zoom to decrease the font size.

[pmarsh-scottlogic]

Hi @Kenny4297

We're closing the PR for now since you won't have time to work on it, and we'd like to discuss within the team and be sure on what the outcome of the issue should be before work continues. Apologies that the requirements weren't crystal clear. Also in hindsight, it might be a bit in-depth for a "good first issue".

That being said, it was good to have somefresh insight to inform our choices going forward. In particular, we're already working to add some css linting to deal with the 0rem problem :)

If you want to continue work on the issue when you have more free time, then please leave us a comment and we'll happily discuss!

Happy new year

[Kenny4297]

Thanks again for letting me work on it! I'll reach back out if I get some time for either this issue or others!