Improve responsive mode for mobile devices #3 #9
Conversation
mrbjnb
commented
Nov 3, 2023
•
mrbjnb
commented
- Add reset.css for reseting the format of all browser.
- Improve responsive mode on mobile devices by adjusting the margin and font-size of header and quote.
- Make the same changes with smaller size.
site/reset.css
Outdated
| q { | ||
| quotes: none; | ||
| } |
There was a problem hiding this comment.
This removes the quotation marks, which I think should be added back.
There was a problem hiding this comment.
I will add it back to the index.css. Because this is the default reset.css for reseting every browser to 0 so I rather not touching it.
There was a problem hiding this comment.
Good work! I'll just remark here that reading Eric Meyer's website, he did intend for the reset.css script to be tweaked as needed! So going forward we should feel free to tweak about as we please.
I don't particularly recommend that you just use this in its unaltered state in your own projects. It should be tweaked, edited, extended, and otherwise tuned to match your specific reset baseline. Fill in your preferred colors for the page, links, and so on.
There was a problem hiding this comment.
Oh I see, then hmm, I should modify the reset.css then.
site/index.css
Outdated
| font-size: 1.2em; | ||
| } | ||
|
|
||
| #aquapants-quote { |
There was a problem hiding this comment.
I think this is a good implementation, but I don't like how it looks on taller screens (too crowded). Can we use a media query to only apply this rule to mobile horizontal screens (ie. max-height is 620px)?
There was a problem hiding this comment.
Yes I can do that, I thought how much I can put in that max-width:620px query 😅 cause that that what the requirement is for (landscape mode)
There was a problem hiding this comment.
Ah I see, maybe I should have commented that block. When I added the max-width:620px query, I meant for that to be portrait mode. The idea of using 620px as a breakpoint was loosely based on this article and some arguments I read on StackOverflow regarding common breakpoints.
I think since landscape mode is just rotated portrait-mode, we should simply treat it with a max-height:620px media query. Your current implementation does not apply the style on most landscape-orientation phones, which often have a width (ie. the height of the phone in portrait mode) that is larger than 620px.
|
Also, FYI, it is good practice to describe what changes you made and why you made them in the Pull Request description. This helps reviewers read your code and also helps future contributors come back to your PR and understand why we did things the way we did. https://www.freecodecamp.org/news/how-to-write-a-pull-request-description/ No need to go into as much detail as the articles say, but some commentary would help a lot. This is also partially our fault because we haven't provided a template yet. I'll make sure we do that! |
|
Also, when opening a PR that is addressing an existing issue in Github, it's particularly useful to mark the pull request using a simple comment like "closes #3" or alternatively, use the UI. More instructions here: |
Added portrait mode, adjusted the size in landscape mode and added quotes back (reset.css remove it)
mrbjnb
changed the title
mrbjnb
changed the title
|
Lmk if I need to add anything more. |
|
@mrbjnb Great improvements! Just a few more suggestions for cleaning it up before we merge and deploy. Also note, on iPhone 12/13 portrait-mode (375/812) the quote looks a bit squished on the sides. We could probably solve this by adding a margin on both sides (I'm guessing the added css reset removed whatever margins used to be there). Screenshot: |
mrbjnb
changed the title
mrbjnb
changed the title
|
Oh I tried to change the name of the branch, I guess it doesn't work xd |
|
@mrbjnb your changes are officially deployed to aquapants.org! |
