Skip to content

Unify rendering TileLayout #4532

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

Merged
merged 6 commits into from
Jul 24, 2023
Merged

Unify rendering TileLayout #4532

merged 6 commits into from
Jul 24, 2023

Conversation

kikostadinov
Copy link
Contributor

@kikostadinov kikostadinov self-assigned this Jun 7, 2023
@kikostadinov kikostadinov force-pushed the unify-rendering-tilelayout branch 2 times, most recently from 45406e8 to 06b1fd3 Compare June 7, 2023 13:33
@epetrow epetrow added this to the 2023.3 milestone Jun 8, 2023
@epetrow
Copy link
Contributor

epetrow commented Jun 9, 2023

Just a few things I noticed so far:

  • Why are we removing the <div class="k-card-title></div> element from the rendering? If it is intentional this leads to a smaller font size of the header -> 14px which differs from the design -> 16px
  • Since we are unifying the rendering, do we need product specific tests e.g. tilelayout-react?
  • If we are removing the k-cursor-move class from the TileLayout header in the reference rendering maybe it is a good idea to add a separate test to showcase it? It was changed from k-cursor-grab as part from this effort
  • I don't think we should add both k-resize and k-resize-x, k-resize-y classes on the same element e.g. k-resize k-resize-x
  • In general when passing string props, we tend to wrap them in double quotes e.g. columnWidth="1fr"
  • I think the commit test(layout) should be changed to test(tilelayout) since we are modifying the TileLayout tests

@kikostadinov kikostadinov force-pushed the unify-rendering-tilelayout branch from 06db941 to d90034f Compare June 13, 2023 13:31
@kikostadinov kikostadinov requested a review from epetrow June 13, 2023 13:45
@kikostadinov kikostadinov force-pushed the unify-rendering-tilelayout branch from 96c0c81 to cf70146 Compare June 30, 2023 10:19
@kikostadinov kikostadinov force-pushed the unify-rendering-tilelayout branch from 9b5cfc2 to 8923d22 Compare July 7, 2023 13:08
@kikostadinov kikostadinov requested a review from Juveniel July 7, 2023 13:09
@epetrow epetrow added C: TileLayout Enhancement New feature of an existing functionality or an improvement of an existing functionality. labels Jul 18, 2023
@Juveniel Juveniel marked this pull request as ready for review July 20, 2023 05:01
@Juveniel Juveniel requested a review from a team as a code owner July 20, 2023 05:01
@epetrow epetrow force-pushed the unify-rendering-tilelayout branch from 1ebf065 to 633c9bc Compare July 21, 2023 13:48
@Juveniel Juveniel merged commit 6e9fc0f into develop Jul 24, 2023
@Juveniel Juveniel deleted the unify-rendering-tilelayout branch July 24, 2023 05:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: TileLayout Enhancement New feature of an existing functionality or an improvement of an existing functionality.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants