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

[feat] <rh-footer> add font-size revert instead of px value for #312

Closed
heyMP opened this issue May 24, 2022 · 4 comments
Closed

[feat] <rh-footer> add font-size revert instead of px value for #312

heyMP opened this issue May 24, 2022 · 4 comments
Assignees
Labels
feature New feature or request for dev Ready for development needs discovery Needs discovery

Comments

@heyMP
Copy link
Member

heyMP commented May 24, 2022

Description

rh-footer explicitly sets the font-size of the :host component to 18px. This was a bandaid to account for sites that had different root font-sizes.

Suggested solution

We should do this instead:

What we came up with was reverting the font-size: revert on :host will set the components base back to the users setting.  I figure we can trap door this with var(--rh-component-base-font-size, revert) or whatever

cc: @zeroedin @markcaron

@heyMP heyMP added feature New feature or request for dev Ready for development labels May 24, 2022
@heyMP heyMP self-assigned this May 24, 2022
@heyMP heyMP changed the title [feat] Add font-size revert instead of px value for rh-footer [feat] rh-footer add font-size revert instead of px value for May 24, 2022
@heyMP heyMP changed the title [feat] rh-footer add font-size revert instead of px value for [feat] <rh-footer> add font-size revert instead of px value for May 24, 2022
@heyMP
Copy link
Member Author

heyMP commented May 25, 2022

@zeroedin is checking on the best corse of action on this.

@heyMP heyMP added needs discovery Needs discovery and removed for dev Ready for development labels May 25, 2022
@bennypowers
Copy link
Member

do we need a variable for this? Users that want to change the default should just

rh-footer {
  font-size: 25em; /* 🌶 */
}

@zeroedin
Copy link
Collaborator

zeroedin commented May 25, 2022

The fix I worked in for rh-secondary-nav resets the font-size to user preference, then uses em's internally to style component sizing. I had originally used rem but spandx exposed the issue of RHDC not using user-specified font sizing.

:host {
  font-size: var(--rh-secondary-nav-base-font-size, initial);
}

This resets to initial but also allows a trap door if someone is adamant about setting that value.

As for your question @bennypowers, do we need a variable? No probably not. Your solution is more valid just override directly in light dom and skip the CSS variable.

@methomps methomps moved this to Backlog in Red Hat Design System Jun 14, 2022
@methomps methomps added the for dev Ready for development label Jun 14, 2022
@heyMP heyMP moved this from Backlog to In Progress in Red Hat Design System Jun 23, 2022
@methomps methomps moved this from In Progress to Review in Red Hat Design System Jun 28, 2022
@bennypowers
Copy link
Member

Closed by #406

Repository owner moved this from Review to Done in Red Hat Design System Jul 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request for dev Ready for development needs discovery Needs discovery
Projects
Status: Done ☑️
Development

No branches or pull requests

4 participants