CJSCommonPlatform / ngGovUk

****** DEPRECATED ********
Other
1 stars 8 forks source link

P Tag is larger than every other element! #51

Open jamesbirrellgray opened 8 years ago

jamesbirrellgray commented 8 years ago

It seems the p tag is 1.19em which is larger that a-tags etc.....?

screen shot 2016-04-06 at 11 16 18
vygis commented 8 years ago

@jamesbirrellgray this seems to be a design decision...

screen shot 2016-04-07 at 10 18 05

It wouldn't be hard to change but then we should amend the x, xs etc classes accordingly and make sure we don't break stuff in the existing apps.

jamesbirrellgray commented 8 years ago

@vygis why is an a tag smaller by default - it makes no sense!

jamesbirrellgray commented 8 years ago

@vygis can we make the change either to the default text (make 19px) so it applies to all elements - span, a etc? or update the p to start at the smae as all other elements as susggested

vygis commented 8 years ago

19px seems excessive, so I suppose it's best to remove the size rule for the default <p> and let it inherit from the body, and then update the smaller p classes accordingly. Let's get the new guys on this :)

jamesbirrellgray commented 8 years ago

@pabloibanezcom this is actually a strange one! the edge case is <li><a>foo</a><p>bar</p><li> - the console says that they are the same size, however the p tag is bigger - maybe play with in the ngGov docs to see if you can replicate

pabloibanezcom commented 8 years ago

@jamesbirrellgray OK, I'm going to see what's the issue

jamesbirrellgray commented 8 years ago

<ul class="row list-unstyled"><li><a>foo</a><p>bar</p><li></ul>

pabloibanezcom commented 8 years ago

I think the problem is that em transformation is applied twice for p. It gets transformated first as it's inside a ul element (this also applied to a) and then as it's a p element (here is where is the difference with a or other elements).

One suggestion could be excluding p from general ul rule but I'm going to think about it.

If anyone wants to know a bit more about this feel free to take a look at this http://www.sitepoint.com/css-font-sizing-tutorial/

pabloibanezcom commented 8 years ago

I'm still not 100% sure about the desired result. Must both a and p to be the same size in all conditions? Are you sure you want to dicrease the font size of the p elements?

vygis commented 8 years ago

Maybe this would be a good opportunity to review sitewide typography with UX?

pabloibanezcom commented 8 years ago

Yes. If we dicrease the current p size by removing the rule the difference between lead parapraph and the standard one will be quite big in my opinion.

jamesbirrellgray commented 8 years ago

Lets review this tomorrow

pabloibanezcom commented 8 years ago

Hi James,

Any update about this? As soon as I know what's the desired sized for p I will create the commit.

pabloibanezcom commented 8 years ago

Finally I removed the font transformation for regular paragraph.

https://github.com/CJSCommonPlatform/ngGovUk/commit/235d68e00936dfb05a1e606d7762cdad3cc98232