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

Header: Improve logo, resizing and styles #13170

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

drummer83
Copy link
Contributor

@drummer83 drummer83 commented Feb 20, 2025

What? Why?

This is a simple improvement of our email header.

Using the site's main logo instead of the footer logo allows for better contrast on the gray header background (dark text on gray instead of white text on gray).

Before:
ksnip_20250216-184848

After:
ksnip_20250216-190129

The table's background color is defined in the parent table already, it's not needed in the header itself again.

Limit the max width and max height of the logo and resize the logo accordingly instead of forcing it to squeeze into a fixed size

Before:
grafik

After:
grafik

Update specs

What should we test?

  • Create any email within the app.
  • Check the displayed header and logo in the

Release notes

Changelog Category (reviewers may add a label for the release notes):

  • User facing changes
  • API changes (V0, V1, DFC or Webhook)
  • Technical changes only
  • Feature toggled

The title of the pull request will be included in the release notes.

Dependencies

Documentation updates

Use the sites main logo instead of footer logo to allow for better contrast on gray background (dark text on gray instead of white text on gray)

The table's background color is defined in the parent table already

Limit the max width and max height of the logo and resize the logo accordingly instead of forcing it to squeeze into a fixed size

Update specs
@drummer83 drummer83 added the user facing changes Thes pull requests affect the user experience label Feb 20, 2025
@drummer83 drummer83 self-assigned this Feb 20, 2025
@drummer83 drummer83 marked this pull request as ready for review February 20, 2025 08:13
@drummer83
Copy link
Contributor Author

We should notify instance managers about this change so they can check their logo.

Copy link
Member

@dacook dacook left a comment

Choose a reason for hiding this comment

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

Yay, nice work! ✨

Copy link
Member

@mkllnk mkllnk left a comment

Choose a reason for hiding this comment

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

Great! I love your little contributions with big impact.

Just wondering about the CSS change.

Comment on lines +124 to +127
.middle {
vertical-align: middle;
}

Copy link
Member

Choose a reason for hiding this comment

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

I don't see this CSS class used anywhere. Did you commit this accidentally?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
user facing changes Thes pull requests affect the user experience
Projects
Status: In Progress ⚙
Development

Successfully merging this pull request may close these issues.

3 participants