NewUI: make card heights consistent

[Sudarshan-21]

closes #117

• Consistent card heights achieved
• Settle the open button to the bottom of card.

Description
In this PR I changed <footer class="pt-2 pb-3"> to <footer class="mt-2 mb-1">. The reason for this particular change is that: padding in the component is not needed. we just need to have some space between different elements which can be achieved by margin(which is a preferred way).

[benoit74]

@Sudarshan-21 thank you! Once this PR is ready, please share here a screenshot of the before / after situation so that our PM can decide if this makes sense or not

[Sudarshan-21]

@Sudarshan-21 thank you! Once this PR is ready, please share here a screenshot of the before / after situation so that our PM can decide if this makes sense or not

sure.

[Sudarshan-21]

Before

After

@benoit74 you can review this design? The basic requirements we discussed in the previous conversation are achieved here.

[benoit74]

@Popolechien do you prefer the UI "before" or "after" this change?

The idea is that in some rare occasions (not encountered so far on real channels, only on test stuff where I merged content from multiple channels into a test channel), the topic image might not be always the same height, and it leads to a weird UI.

@Sudarshan-21 considers that current new UI ("before" screenshot above) is not adequate and is proposing to change it to the "after" UI where all cards have the same height.

Personally I tend to prefer the "before" UI because images are never distorted, while they are in the "after" situation (and cropping instead of distorting them would not help much since it would cut some text at a random position, since we have no control on where important content is placed in the image). I agree it is not perfect, but I have no clue about how to make it perfect without ensuring that images in the channel we use are always the same height for a given topic (which is most probably the case 99% of the time).

I'm quite sure that distorting images will cause more users to complain than having cards of various heights.

WDYT?

[Popolechien]

The idea is that in some rare occasions (not encountered so far on real channels

@benoit74 So basically implementing it is not likely to have any material effect on current display (except for rare and yet-unseen-in-the-wild-occasions), correct?

Regarding the distortion: how hard is it to set the default height to that of the tallest image, and for all other images adapt the text region to make up for it accordingly (which means that image height would not be hardcoded but relative to the available set)?

In either case (distortion or not), having cards at the same height would be my preferred path going forward.

[benoit74]

So basically implementing it is not likely to have any material effect on current display (except for rare and yet-unseen-in-the-wild-occasions), correct?

Correct

Regarding the distortion: how hard is it to set the default height to that of the tallest image, and for all other images adapt the text region to make up for it accordingly (which means that image height would not be hardcoded but relative to the available set)?

@Sudarshan-21 could you please give it a try and share a screenshot? I like the idea, this is probably the best compromise to have no distortion but still consistent card height

In either case (distortion or not), having cards at the same height would be my preferred path going forward.

Thank you!

[Sudarshan-21]

Regarding the distortion: how hard is it to set the default height to that of the tallest image, and for all other images adapt the text region to make up for it accordingly (which means that image height would not be hardcoded but relative to the available set)?

In either case (distortion or not), having cards at the same height would be my preferred path going forward.

@Popolechien In the previous design consideration before finally ending up with this proposed design, I considered keeping the images to their actual heights and maintaining their aspect ratios(so that there is no distortion or cropping of images).
But the problem with that type of design is that the difference between the height of shorter images and the tallest image is so large that it looked bad in UI to me. So, to minimize the difference to maintain a relatively smaller difference, I added a max-height style to the card-image. To achieve your proposed design, I just need to change that style for the card-image.

Personally I tend to prefer the "before" UI because images are never distorted, while they are in the "after" situation (and cropping instead of distorting them would not help much since it would cut some text at a random position, since we have no control on where important content is placed in the image).

@benoit74 regarding your concern about the cropping of images, they are never cropped in the proposed UI, but yes, they are distorted(shrink property makes them shrink vertically if their height exceeds beyond 60% of the card height).

I would like to have your views on the design after the explanation. If you want, I would be happy to provide you with the UI with your improvisations.

[benoit74]

regarding your concern about the cropping of images, they are never cropped in the proposed UI,

I know, I just mentioned it as it could be seen as another alternative (but not compelling from my PoV)

To achieve your proposed design, I just need to change that style for the card-image.

If it is simple, please do so and share a screenshot. I don't think it is easy to understand your concern (which might be right) without a screenshot, at least I find it a bit vague

[Sudarshan-21]

If it is simple, please do so and share a screenshot. I don't think it is easy to understand your concern (which might be right) without a screenshot, at least I find it a bit vague

@benoit74 The following is the UI after the suggested improvisation

[benoit74]

@Popolechien does your thumb up means the last proposition is your preferred solution?

I like it indeed, I would just ensure there is some space between the text and the "Open" button in all circumstances (e.g. "Caperucita Roja" is a bit too close to "Open" button)

[Popolechien]

Yep, looks good to me (and agree on the padding requirement)

[benoit74]

@Sudarshan-21 please push the necessary modifications to achieve last screenshot (including the padding between text and button) and ask for a new review

[Sudarshan-21]

@benoit74 all of the required changes are made, you can review this PR

[benoit74]

Your PR is not complete, you've made modifications only to the "default" cards, but not to "topic" and "more" cards, which have to be adapted as well to have consistent styling on all cards.

See conversations for additional details

[Sudarshan-21]

Your PR is not complete, you've made modifications only to the "default" cards, but not to "topic" and "more" cards, which have to be adapted as well to have consistent styling on all cards.

@benoit74 there were no styles given to the topic and more classes, so do we want all the styles of card classes to be applied to these two classes?

[benoit74]

@benoit74 there were no styles given to the topic and more classes, so do we want all the styles of card classes to be applied to these two classes?

I'm not speaking about the topic and more classes, I'm speaking about the Vue/HTML code. Since for instance the "main" div all have at least the card class they inherit most of your changes but what is missing is other changes like the additional h-100 and open-badge classes (btw, it shows that open-badge class should be renamed to something more generic since we want to apply this class to topic and more badges as well). Is it clearer?

[Sudarshan-21]

all have at least the card class they inherit most of your changes but what is missing is other changes like the additional h-100

I have moved the h-100 style to the card class itself. so this style will be applied whenever the card class is present.

btw, it shows that open-badge class should be renamed to something more generic since we want to apply this class to topic and more badges as well)

I have renamed it, please check.
Is there anything more @benoit74?

[benoit74]

badge-wrapper class is still missing for "MORE" and "EXPLORE" buttons

[Sudarshan-21]

@benoit74 I have made the required changes, let me know if there are more things to be taken care of.

[benoit74]

LGTM, works well when the 3 kind of cards are present now:

Thank you!

[benoit74]

I just squashed all commits into one and enhanced the commit message