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

BUGFIX: Column spacing and missing bold font #461

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

crydotsnake
Copy link
Member

@crydotsnake crydotsnake commented Oct 21, 2022

This pull request provides a few bugfixes for the Neos website. Previously it was not possible to mark text as Bold in the word processor, because the required font file was not loaded. I went through the layout afterwards, and everywhere where the text was Bold afterwards (like in the navigation) I set the font-weight to regular, so the layout wouldn't break. Also, in the text with image element, there was no space between the image and the text when I placed the image either on the left, or right side. I also added a small space between the text and the icon in the ImageTeaser, so that it doesn't look so squeezed on top of each other. But I have summarized my changes in screenshots.

Before Screenshots

neos-text-with-picture-element-without-spacings

showcase-without-spacing

After Screenshots

neos-text-with-picture-element-spacings

showcase-with-spacing

@Sebobo Sebobo changed the title FEATURE: small layout improvements BUGFIX: Column spacing and missing bold font Oct 21, 2022
@Sebobo
Copy link
Member

Sebobo commented Oct 21, 2022

Thx! Looks good by reading but will test it locally.

I updated the title as its more like a bugfix and should be more precise.

@Sebobo Sebobo added the bug label Oct 21, 2022
@crydotsnake
Copy link
Member Author

crydotsnake commented Oct 21, 2022

Thx! Looks good by reading but will test it locally.

I updated the title as its more like a bugfix and should be more precise.

Thanks! 💙

Yes, in hindsight you are right.

I updated also the description above

@@ -79,6 +79,7 @@
margin-bottom: $half-spacing+px;
font-size: $base-font-size*1.3px;
line-height: $base-line-height*1px;
font-weight: $regular;
Copy link
Member

Choose a reason for hiding this comment

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

This is not necessary. We need to adjust the base mixin h() to use $regular. Additionally you can remove the font-weight overrides in the mixins h5 and h6 as they would break the current website styling.

You can check the impact of this change e.g. on the schedule page of the NEOSCON.

@crydotsnake crydotsnake force-pushed the feature/small-layout-improvements branch from fa3bd92 to 3d6bdd5 Compare December 28, 2022 16:26
@Sebobo
Copy link
Member

Sebobo commented Apr 26, 2023

@crydotsnake did you implement all requested changes by @ahaeslich

@crydotsnake
Copy link
Member Author

Sadly not. Unfortunately I have totally lost sight of that

@ahaeslich
Copy link
Member

@Sebobo is this MR still relevant regarding that you are working on the website right now?

@Sebobo
Copy link
Member

Sebobo commented Jul 25, 2023

@ahaeslich it will have some conflicts as I resolved the font problem with a variable font type, but I didn't touch the spacing part.

So rebasing and targeting the development branch would be best.

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

Successfully merging this pull request may close these issues.

3 participants