Add options to configure thumbnail alignment

[AllanOXDi]

Description

This PR introduces a new thumbnailAlign prop to the KCard component, enabling developers to configure the alignment of the thumbnail area within the card. This newthumbnailAlignprop accepts 'left' and 'right' values, and it decides on which side is the thumbnail area displayed, particularly in the horizontal layout.

Issue addressed

#706

With the left and right align options

Changelog

[#706]

• Description: Add new alignment options to KCard
• Products impact: new alignment
• Addresses: #701
• Components: KCard
• Breaking: N0
• Impacts a11y: No
• Guidance:
  [Add options to configure thumbnail alignment #706]: Add options to configure thumbnail alignment #706

Testing checklist

• Contributor has fully tested the PR manually
• If there are any front-end changes, before/after screenshots are included
• Critical and brittle code paths are covered by unit tests
• The change is described in the changelog section above

Reviewer guidance

• Is the code clean and well-commented?
• Are there tests for this change?
• Are all UI components LTR and RTL compliant (if applicable)?
• Add other things to check for here

After review

• The changelog item has been pasted to the CHANGELOG.md

Comments

[AllanOXDi]

@MisRob and @AlexVelezLl -Noting that Support for RTL will be address separately #665

[MisRob]

Hi @AllanOXDi, I'd like to suggest one update that will help with the following

• thumbnailAling as is now takes effect only in horizontal layout with large thumbnail, and the whole implementation is very tightly coupled to this use case. Even though this is the first motivating use-case, there's no need to limit it to this particular case, by means of API as well as internal implementation. It is intended to take effect in any case where it makes sense, for example in the horizontal layout with small thumbnail.
• Keeping the style organization simple as best as we can by limiting the number of conditions and keeping them at a single place, in both JavaScript and styles sections.

I think something similar to the following could help to open it up and make a bit clearer as well:

1. Renaming rootClass to rootClasses and using it on root element as :class="rootClasses"
2. Updating stylesAndClasses existing conditions to something like:

stylesAndClasses() {
      const commonRootClasses = [
         'k-card',
        this.thumbnailAlign === ThumbnailAlign.LEFT ? 'thumbnail-left' : 'thumbnail-right'
      ]
   
      if (this.layout === 'vertical' && this.thumbnailDisplay === 'large') {
        return {
          rootClasses: [...commonRootClasses, 'vertical-with-large-thumbnail'],
          ...
      }
      if (this.layout === 'vertical' && this.thumbnailDisplay === 'small') {
        return { rootClasses: [...commonRootClasses, 'vertical-with-small-thumbnail'],
         ...
        }
      }
     // update `rootClass` in this way for all other existing branches
    },
  },

This ensures all thumbnail layouts have information on what alignment we've chosen.

3. In styles, just updating relevant layouts, currently the following two. All styles related to the thumbnail alignment are grouped within them, in these two new classes:

.horizontal-with-large-thumbnail
   &.thumbnail-left {
     
   }

  &.thumbnail-right {
     
   }
}

.horizontal-with-small-thumbnail
   &.thumbnail-left {
     
   }

  &.thumbnail-right {
     
   }
}

If you can think about further improvements, I welcome them, this is one of the approaches I could think of right now.

[AlexVelezLl]

I think it would be good to have this variation in the documentation too, showing some examples. What do you think? @AllanOXDi @MisRob

[AlexVelezLl]

Just wondering if are these extra spaces enforced by the linter? 😅

[MisRob]

@AllanOXDi I think this looks better now, thank you. I appreciate you simplified my original suggestion even further 👍 Let's just finish cleanup of the previous implementation.

[MisRob]

I think it would be good to have this variation in the documentation too, showing some examples. What do you think? @AllanOXDi @MisRob

@AlexVelezLl @AllanOXDi Yes

[MisRob]

@AllanOXDi [required] I've just noticed that the thumbnail alignment still doesn't take effect for the horizontal layout with the small thumbnail which is expected behavior. Using the approach you've chosen

BaseCard :class="['k-card', rootClass, thumbnailAlignClass]">

we can achieve that by

stylesAndClasses() {
      if (this.layout === 'horizontal' && this.thumbnailDisplay === 'large') {
        return {
          rootClass: 'horizontal-with-large-thumbnail',
          thumbnailAlignClass: `thumbnail-align-${this.thumbnailAlign}`
          ...
        }
      }
      if (this.layout === 'vertical' && this.thumbnailDisplay === 'small') {
        return {
           rootClass: 'vertical-with-small-thumbnail',
           thumbnailAlignClass: undefined
           ...
        }
      }
     if (this.layout === 'horizontal' && this.thumbnailDisplay === 'small') {
        return {
           rootClass: 'vertical-with-small-thumbnail',
           thumbnailAlignClass: `thumbnail-align-${this.thumbnailAlign}`
           ...
        }
      }
    },
...
...
...
  },

I think this is a nice merging of both approaches that will most importantly resolve the remaining issue for small thumbnail. Additionally, keeping everything in stylesAndClasses as a single source of truth will help you (and potentially any future devs) to understand what is applied exactly to what layouts - since we can now have it defined in the existing conditional branches rather than having to add the same very conditional branches to thumbnailAlignClass computed property which is removed in this proposal.

After this is setup, you can add new styles for .horizontal-with-small-thumbnail using the very same structure you used for styling .horizontal-with-large-thumbnail, that is

.horizontal-with-small-thumbnail {
  &.thumbnail-align-left {

  }

  &.thumbnail-align-right {

  }
}

[AllanOXDi]

Hi @MisRob ,just noting that the KTextTruncatoron different layouts would render different tiltle lines. And the solution was to re-introduce a titleLine prop that gives it a default lines to handle it. When the KCard prop(titleLines) is not specified it defaults to 2lines.

[AllanOXDi]

Also, I have added a few different layouts on the playground for testing

[MisRob]

Hi @AllanOXDi, good progress! I think we're almost there. Left a few minor cleanup comments. There's only two changes I consider required:

• (1) The spacer value regression - however if we revert here, I think the issue it attempted to solve could be addressed in another PR since it's not related to thumbnails
• (2) Not mixing two techniques to achieve left/right alignment of slots. We can simply utilize flex features for aligning items as that's what is already in place. Overriding it with margins is not needed and on some occasions it creates conflicting or unnecessary styles. Not that using the margins technique would be generally bad - it's definitely valid one! My concern is about not mixing the two approaches. Let me know if you wanted to chat or had questions.

I think after this is done, all should be in place for merge.

[MisRob]

[required] Was the change from 24 to 12 intentional? I think we will need to keep it due #709.

If this was part of the fix related to the lack of space on cards, that could be fixed by increasing the overall height in all layouts where it's needed. The overall heights are defined in the css part, for example:

 .horizontal-with-large-thumbnail {
    ...
    height: 240px;
    ...

Same applies to $spacer.

I think it'd be fine to revert to 24 in this PR and then do the height fix in a follow-up PR if it'd be better, feel free to decide.

[MisRob]

[required] I believe that titleAlignStyle can be removed completely after you utilize switching between flex-end and flex-start. Explained in mode detail in other comments.

[AllanOXDi]

I think we need still need this to handle switching between the title.

[MisRob]

Response here #706 (comment)

[MisRob]

Hi @MisRob ,just noting that the KTextTruncatoron different layouts would render different tiltle lines. And the solution was to re-introduce a titleLine prop that gives it a default lines to handle it. When the KCard prop(titleLines) is not specified it defaults to 2lines.

Yes, thank you! I think this was my oversight, I don't know why I did it honestly.

[MisRob]

Also, I have added a few different layouts on the playground for testing

This helps a lot - thank you

[MisRob]

Well done! Good to go :)

I am observing two issues on the playground page, none of them blocking and even related to this work so I will open follow-ups later on.