govau / design-system-components

🛠 Component code and tests for the Australian Government design system
https://auds.service.gov.au
MIT License
739 stars 111 forks source link

Card list height match issue on older browsers #822

Open sukhrajghuman opened 5 years ago

sukhrajghuman commented 5 years ago

I think we need to implement flexibility](https://github.com/jonathantneal/flexibility) to solve this?

image

sukhrajghuman commented 5 years ago

I couldn't get this to work with the latest version of flexibility. It seems to be an unmaintained library, with the last update being in April 2018. I think we will just remove the flex styling for the match height class for IE using the .ie9 selector

petrillek commented 5 years ago

I was wondering about the CSS structure used for match height (Flex inside Flex). The second flex is not needed and require you to provide extra styling on au-card to revert some defaults.

So instead of:

.au-card-list.au-card-list--matchheight li {
    display: -webkit-box;
    display: -ms-flexbox;
    display: flex;
}
.au-card-list.au-card-list--matchheight .au-card {
    display: block;
    -webkit-box-orient: vertical;
    -webkit-box-direction: normal;
    -ms-flex-direction: column;
    flex-direction: column;
}

why not use just:

.au-card-list.au-card-list--matchheight .au-card {
    display: block;
    height: 100%;
}

That should be supported in IE9 and will do what we need.

sukhrajghuman commented 5 years ago

Hey @petrillek thanks for the suggestion!

the current mark up is something like:

<ul class="au-card-list au-card-list--matchheight row">
  <li class="col-sm-4 col-xs-6>
    <div class="au-card">...</div>
  </li>
</ul>

Adding height:100% to the card will not make a difference from my experiments, since the parent is the <li> and all list items are contained inside a single row.

If we alter the mark up so that we place each row of cards in a <ul>, this could work. See below example:

https://codepen.io/sukhrajghuman/pen/GVmrRE?editors=1000

However in this case we lost the semantic benefit of all related items being inside a single <ul>... still experimenting with this one

petrillek commented 5 years ago

Hi @sukhrajghuman, thanks for looking into it. I've look at your pen and cannot see any custom styling there, so maybe a wrong URL? Anyway I have created a fiddle of my own to show what I mean. I coppied the markup from Example at https://designsystem.gov.au/components/card/#card-lists and use directly the relevant CSS output from modules gov.au/card and gov.au/grid. My changes are done on lines 157, 162, 163 – does that work for you?

https://jsfiddle.net/bighead01/r48yabng/16/

sukhrajghuman commented 5 years ago

Hey @petrillek the url I sent is the default styling as per the DS (the css is hidden). It's just that I've split 8 cards into 2 lists, each contained within their own row.

Your solution cleans up our code a bit so thanks for that!

From some testing in ie8 I'm still getting the same problem though.

image

sukhrajghuman commented 5 years ago

Splitting <ul> into two lists (each in their own <div class="row">) in ie8 gets this result.

image

This is what we would want it to look like (of course in a single <ul>)

petrillek commented 5 years ago

Have a look at this version https://jsfiddle.net/bighead01/r48yabng/25/ Ive simulated the IE8 behaviour, but not tested it in as I don't have any device running it nearby. The important lines are 151–153.

The problem here is that IE8 don't support flex in any way – the LI will not get the equal height, thus the 100% height will not work. And only the float property from grid will take effect. But as the Card 2 has more content it will "catch" other cards in the flow.

Solution is to add a clear to force the wrapping at corect point. The problem of this solution is that if you have a grid of n items the clear needs to be on n+1 item. And as IE8 does not support nth-child it will be much harder to do that.

I'm afraid that without JS you will not be able to solve it on IE8. :(

My long term question is, why we need to support ancient browser like IE8? It is unsecure, not supported for over 3 years now. It forces us to do weird workarounds. I'm happy to have this discussion on slack. ;)

sukhrajghuman commented 5 years ago

I'm afraid that without JS you will not be able to solve it on IE8. :(

I think I'm starting to reach this conclusion as well. And I really don't want to ship this component with javascript 😅

My long term question is, why we need to support ancient browser like IE8?

We discussed browser support in our summer meetup earlier this year. The way we thought about it was that there are a very small percentage of users on browsers <ie10. But if you multiply that by a large number of hits across government sites, it works out to be a lot.

I'm happy to discuss bring this up with the team again. I was thinking of doing what gov.uk do, and have an alert message at the top for anybody using <ie10, suggesting they upgrade.

This browser support discussion is also on our community forums.