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

PLP indicators disrupt container outlines in Firefox #2619

Closed
tomgreenfield opened this issue Jan 9, 2020 · 6 comments · Fixed by #2906
Closed

PLP indicators disrupt container outlines in Firefox #2619

tomgreenfield opened this issue Jan 9, 2020 · 6 comments · Fixed by #2906

Comments

@tomgreenfield
Copy link
Contributor

Subject of the issue/enhancement/features

Container outline bounds are increased in height in Firefox due to the use of inline-block and whitespace between sibling elements.

Your environment

  • Framework 5.1
  • Firefox 72.0.1
  • Windows 10

Steps to reproduce

  1. Go to https://adaptlearning.github.io/v5demo/#/id/co-10
  2. Run the following in the console:
$(".c-60").css({ outline: "1px solid black", backgroundColor: "grey" });

Expected behaviour

Top of container outline should be equal to its actual top.

Actual behaviour

Container outline is pushed several pixels higher.

Screenshots (if you can)

plp

@akshay-bhavsar
Copy link
Contributor

@tomgreenfield ,

There are two ways to solve this issue.
one could be Use of border property instead of using outline,
else we can remove "line-height" property of "component__title" class, but that causes little disruption in height of component title text.

what do you think?

@tomgreenfield
Copy link
Contributor Author

Hi @akshay-bhavsar, the proper way to fix this is to simply omit the whitespace in the component template:


becomes:

      {{~/unless~}}

@akshay-bhavsar
Copy link
Contributor

works well!!

I will Raise PR for the same.

@akshay-bhavsar
Copy link
Contributor

PR: #2666

@moloko
Copy link
Contributor

moloko commented Mar 9, 2020

fixed via 5e39624

@moloko moloko closed this as completed Mar 9, 2020
@moloko
Copy link
Contributor

moloko commented Apr 23, 2020

actually - does this not also need applying to article & block titles as well?

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 a pull request may close this issue.

4 participants