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

Fix alignment issue with default container width #22

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

Conversation

iFlameing
Copy link
Member

@tisto @sneridagh Screenshots

Screenshot 2024-02-27 at 8 03 51 PM

Copy link
Member

@sneridagh sneridagh left a comment

Choose a reason for hiding this comment

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

Did you check on mobile? I guess it would be no margin now.

@sneridagh
Copy link
Member

@jackahl also tell me if this is acceptable giving the initial specs of the block, and if it's going to affect existing projects.

@tisto
Copy link
Member

tisto commented Feb 27, 2024

Maybe I don't fully get how flexbox works. However, won't the size of the logo affect this? On the screenshot, it looks pretty good though.

@iFlameing
Copy link
Member Author

Maybe I don't fully get how flexbox works. However, won't the size of the logo affect this? On the screenshot, it looks pretty good though.

No, my current implementation only changes the alignments of logo.

@iFlameing
Copy link
Member Author

@sneridagh screenshots

Screenshot 2024-02-29 at 8 38 36 PM
Screenshot 2024-02-29 at 8 38 17 PM

@iFlameing
Copy link
Member Author

Did you check on mobile? I guess it would be no margin now.

I fixed it now. Margin is already removed.

@iFlameing iFlameing requested a review from sneridagh February 29, 2024 15:10
@iFlameing
Copy link
Member Author

@sneridagh Please merge this.

@sneridagh
Copy link
Member

I tested it with two logos, I don't think this is what we have in mind:

@tisto we can't go full width with the logos, can we meet and specify it a bit better?

image

@sneridagh
Copy link
Member

@jackahl which was the initial spec?

@sneridagh
Copy link
Member

@iFlameing I decided with Timo that this will go in .com only. We will use this solution for blocks >= 3, and the current solution for below this. So close this one, I will add a ticket for the .com site.

@sneridagh
Copy link
Member

@iFlameing I just realised that we didn't implement this. We decided that we will go with this fix (images tight with the container in .com only.

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.

3 participants