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

753 inconsistet fonts and spacing on loop #761

Merged
merged 6 commits into from
May 2, 2024

Conversation

vitorgomateus
Copy link
Contributor

Fixes #753.
Fixes #754.

Changes in this request

  • create consistency in the content block width by changing the limit from the p rule to the general .block-paragraph.
  • add z-index to the left sidebar so that its content doesn't get visually mixed with the footer.
  • new logo's maroon has low contrast with loop's green so it was changed to the all-white logo.

@vitorgomateus vitorgomateus linked an issue Apr 26, 2024 that may be closed by this pull request
@vitorgomateus vitorgomateus requested a review from bbusenius April 29, 2024 15:15
@vitorgomateus vitorgomateus self-assigned this Apr 29, 2024
Copy link
Member

@bbusenius bbusenius left a comment

Choose a reason for hiding this comment

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

The logo looks great and I like your use of the consolidated web-resources folder. I'm still seeing font problems on loop though. The lists still aren't constrained and the font size is smaller:

image

I emptied the cache and hard-refreshed so I don't think I'm seeing a caching issue.

@vitorgomateus
Copy link
Contributor Author

I didn't notice that the Loop SCSS needs to be compressed separately. I wasn't able to run Gulp, so I made 'compressed' changes manually. I also fixed the difference in font-size and line-height, and added a overflow-wrap: break-word; because I saw a long link in this particular example page.

@bbusenius
Copy link
Member

This is looking a lot better. I see the text is constrained now and the line height looks better. I still see a couple of issues though.

  1. The list font is still smaller than the paragraph text. I think this is because the paragraph has font-size: 1.1em but the list is just inheriting font-size: 14px from the body.
  2. I think we need to get rid of this gulp dependency and move this SASS into the compress framework.

Maybe if we address the first issue in this PR, we can open a new PR for the second. Otherwise, we could address them both here. I don't have a strong preference.

@vitorgomateus
Copy link
Contributor Author

It seems these new rules do not apply on the news page type. I'll make new rules to include those as well.
I will create a new issue to get rid of the gulp dependency, or at least create consistency between lib and loop because gulp would be helpful for dev, but it has to work with compress. So I wanna take a little extra time on it.

@bbusenius
Copy link
Member

Nice, this looks good to go!

@bbusenius bbusenius self-requested a review May 1, 2024 17:10
@vitorgomateus vitorgomateus merged commit 4d623a7 into master May 2, 2024
1 check passed
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.

sidebar and footer content are mixed on mobile Inconsistet fonts and spacing on Loop
2 participants