Conversation
github-actions
bot
added
P-Feature: About Us
anonymousanemone
removed
the
status: Help Wanted
Zak234
left a comment
Zak234
left a comment
There was a problem hiding this comment.
The changes you made look good and seem to be working. I'm pretty sure this isn't related to your issue but I noticed when I was checking your code changes through different phone resolutions that for a lot of the other phone resolutions besides iphone X that the wins-card is off-center. Should this be a new issue?
|
@Zak234 Can you attach screenshots of what you are seeing vs the appearance of the mobile view on Figma? |
There was a problem hiding this comment.
Following up on @Zak234 comment. It looks great on desktop/mobile view. It seems the tablet view just aligns to the left.
Click to see Tablet view (iPad) of About page (Accomplishments)
It looks like the the card flexbox converts to column in tablet view and since we are providing a width to the svg, it aligns to the left (start). So maybe add styling for tablet view (line 604-618).
|
|
|
I've centered the image for "below tablet" which is when the screen is under 768px, but should I also center the image for when it is "bigger than tablet" but less than desktop? Here's how it looks: |
anonymousanemone
added
the
status: Help Wanted
|
Ask design team about whether to center or do something else |
|
@anonymousanemone thanks for checking in about this. Please center the card for the "bigger than tablet" view as well. |
|
I've made both of the changes requested above, please re-review! @jbubar @abuna1985 |
macho-catt
left a comment
macho-catt
left a comment
There was a problem hiding this comment.
For the most part, changes on the page card itself looks good. Good job!
I also noticed that you've been working on this for a while, so props to you on that. However, I noticed one other thing that may not be in the jurisdiction of your issue/PR.
@Zak234 noticed it in one of his previous posts, but at a certain BP the parent cards are cutting off. This is due to the fact that flex (or any mobile responsive query) is not applied to the parent divs at specific BPs:
Click to see example
The fix would be to apply flex (or some sort of media query) on the parent divs for each section of the About page.
@Aveline-art @abuna1985 is it better to open up a new issue for this? Otherwise, I would be inclined to approve this issue.
|
@macho-catt Go ahead and create a new issue for adding margin to the parent divs of the About page. Once that is complete, add it to this pr as a comment and approve the pull request. Then I will merge it. Thank you. |
|
Opened a separate issue for the cutting off bug: #2077 |
7 participants
Fixes #1908
What changes did you make and why did you make them ?
Screenshots of Proposed Changes Of The Website (if any, please do not screen shot code changes)
Visuals after changes are applied
Visuals before changes are applied