Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

docs: tablet mode paddings #1572

Merged
merged 7 commits into from
Nov 18, 2020
Merged

docs: tablet mode paddings #1572

merged 7 commits into from
Nov 18, 2020

Conversation

stefanoScalzo
Copy link
Contributor

@stefanoScalzo stefanoScalzo commented Nov 3, 2020 •
edited
Loading

Fixes the padding on the landing page for tablet sizes

@netlify
Copy link

netlify bot commented Nov 3, 2020 •
edited
Loading

Deploy preview for fundamental ready!

Built with commit 0d89212

https://deploy-preview-1572--fundamental.netlify.app

jessmetivier
jessmetivier suggested changes Nov 4, 2020
View reviewed changes
Copy link

@jessmetivier jessmetivier left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey stefano,
Thanks for doing this quick :)

I have one comment:

  1. The arrows are the wrong size (should be like desktop/48 px)
    Arrows are also way below the text and aren't lining up with the side margins (for the tablet grid).

Screen Shot 2020-11-04 at 9 21 38 AM

@stefanoScalzo
Copy link
Contributor Author

Hey stefano,
Thanks for doing this quick :)

I have one comment:

  1. The arrows are the wrong size (should be like desktop/48 px)
    Arrows are also way below the text and aren't lining up with the side margins (for the tablet grid).
Screen Shot 2020-11-04 at 9 21 38 AM

fixed

jessmetivier
jessmetivier suggested changes Nov 4, 2020
View reviewed changes
Copy link

@jessmetivier jessmetivier left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great!

Could you just remove the awkward line break and left-align the text for the intro sentence? It looks awkward and should be consistent with the other sections.
Screen Shot 2020-11-04 at 1 52 23 PM

@stefanoScalzo
Copy link
Contributor Author

Great!

Could you just remove the awkward line break and left-align the text for the intro sentence? It looks awkward and should be consistent with the other sections.
Screen Shot 2020-11-04 at 1 52 23 PM

Fixed

jessmetivier
jessmetivier suggested changes Nov 4, 2020
View reviewed changes
Copy link

@jessmetivier jessmetivier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just noticed this... the arrows are located at the edge of the screen like mobile is designed, they should be aligned with the margins like in the sketch doc:
Screen Shot 2020-11-04 at 5 08 58 PM

@stefanoScalzo
Copy link
Contributor Author

Just noticed this... the arrows are located at the edge of the screen like mobile is designed, they should be aligned with the margins like in the sketch doc:
Screen Shot 2020-11-04 at 5 08 58 PM

I fixed the padding on the arrows, since blogs are also getting refactored completely it is normal that the grid isn't completely aligned with them yet and will be in another pr

jessmetivier
jessmetivier suggested changes Nov 10, 2020
View reviewed changes
Copy link

@jessmetivier jessmetivier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The arrows are now misplaced when in mobile view. They were perfect before.
Screen Shot 2020-11-10 at 9 39 41 AM

When I noted the issues with the arrows, I was just talking about the tablet version.

jessmetivier
jessmetivier suggested changes Nov 13, 2020
View reviewed changes
Copy link

@jessmetivier jessmetivier left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everything looks great! There's just one weird thing that I noticed it:

The spacing is off between the Libraries and Key Features area
Screen Shot 2020-11-13 at 12 53 43 PM

jessmetivier
jessmetivier suggested changes Nov 17, 2020
View reviewed changes
Copy link

@jessmetivier jessmetivier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Stefano,

It's almost good to go. There's just an issue with the arrows, it looks like one side has more padding/less margin than the other.
Screen Shot 2020-11-17 at 12 06 21 PM

@stefanoScalzo
Copy link
Contributor Author

Hi Stefano,

It's almost good to go. There's just an issue with the arrows, it looks like one side has more padding/less margin than the other.
Screen Shot 2020-11-17 at 12 06 21 PM

fixed

jessmetivier
jessmetivier approved these changes Nov 17, 2020
View reviewed changes
Copy link

@jessmetivier jessmetivier left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

C'est bon!

@stefanoScalzo stefanoScalzo self-assigned this Nov 18, 2020
@droshev droshev self-requested a review November 18, 2020 15:28
@stefanoScalzo stefanoScalzo merged commit 4afd2f3 into master Nov 18, 2020
@stefanoScalzo stefanoScalzo deleted the docs-tablet-paddings branch November 18, 2020 15:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jessmetivier jessmetivier jessmetivier approved these changes

@droshev droshev droshev approved these changes

Assignees

@stefanoScalzo stefanoScalzo

Labels
landing page
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@stefanoScalzo @droshev @jessmetivier