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.

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

7393 - Update content width #7405

Closed
wants to merge 1 commit into from
Closed

Conversation

fessehaye
Copy link
Contributor

Closes #7393

@mofodevops mofodevops temporarily deployed to foundation-s-7393-conte-trjm2z September 17, 2021 17:33 Inactive
Copy link

@sabrinang sabrinang left a comment

Choose a reason for hiding this comment

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

@fessehaye Which specific product page URL in the deployment should review? When I click on a product it is Bad Request (400) for me.

@Pomax Pomax temporarily deployed to foundation-s-7393-conte-trjm2z September 20, 2021 17:10 Inactive
@Pomax Pomax temporarily deployed to foundation-s-7393-conte-trjm2z September 20, 2021 17:15 Inactive
@Pomax Pomax temporarily deployed to foundation-s-7393-conte-trjm2z September 20, 2021 17:17 Inactive
@Pomax Pomax temporarily deployed to foundation-s-7393-conte-trjm2z September 20, 2021 17:18 Inactive
@Pomax Pomax temporarily deployed to foundation-s-7393-conte-trjm2z September 20, 2021 17:19 Inactive
Copy link
Collaborator

@danielfmiranda danielfmiranda left a comment

Choose a reason for hiding this comment

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

Hey Simon! Looks good to me, approving!

@sabrinang sabrinang self-requested a review September 20, 2021 21:23
Copy link

@sabrinang sabrinang left a comment

Choose a reason for hiding this comment

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

The text contents/updates/related products should align with the 8 col grid so it seems should be a bit wider:
screencap-grid

(I used a bootstrap grid overlay browser extension to check)

In the figma mockup, you can also go to View > Show Layout Grids for alignment reference

@mofodevops mofodevops temporarily deployed to foundation-s-7393-conte-trjm2z September 22, 2021 17:06 Inactive
@mofodevops mofodevops temporarily deployed to foundation-s-7393-conte-trjm2z September 23, 2021 06:19 Inactive
@mofodevops mofodevops temporarily deployed to foundation-s-7393-conte-trjm2z September 23, 2021 17:26 Inactive
@mofodevops mofodevops temporarily deployed to foundation-s-7393-conte-trjm2z September 23, 2021 21:04 Inactive
Copy link

@sabrinang sabrinang left a comment

Choose a reason for hiding this comment

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

  • the cut out sections can align to the edge of column 2 and 11 for the top section and dotted banner below
  • the content below the dotted banner will be the same width as the content above to align with edge of column 3 and 10 as you have updated it
  • highlighted in pink lines for your reference

image

image

Copy link

@sabrinang sabrinang left a comment

Choose a reason for hiding this comment

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

Cut outs are good now but the text after the dotted banner should also align (similarly to text above to the edge of col 3 and 10):
image

@Pomax Pomax temporarily deployed to foundation-s-7393-conte-vetw5g October 6, 2021 21:11 Inactive
@mofodevops mofodevops temporarily deployed to foundation-s-7393-conte-vetw5g October 7, 2021 05:19 Inactive
Fixed padding

revision

Changes

Updates
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants