Skip to content
This repository has been archived by the owner on Mar 4, 2020. It is now read-only.

fix(layout): remove truncate* props from Layout and ItemLayout #1127

Merged
merged 6 commits into from
Apr 2, 2019

Conversation

bmdalex
Copy link
Collaborator

@bmdalex bmdalex commented Mar 28, 2019

Steps to from BREAKING CHANGES:

We recommend using Flex instead and follow our Layout Components recommendations

However, if you still want to use Layout/ItemLayout, the steps are:

  1. Wrap the element that you need truncated with truncated Text component
  2. Pass that element to Layout/ItemLayout's slot
  3. Add additional CSS as in examples below

Layout

<Layout
  // ...
  end={<Text truncated>My long text that I need truncated</Text>}
  endCSS={{ overflow: 'hidden' }}
/>

See complete code: https://codesandbox.io/s/pm4vn7j0mm

ItemLayout

<ItemLayout
  // ..
  endMedia={<Text truncated>Some very loooooooooooooooooooooooooooong text</Text>}
  endMediaCSS={{ width: '100px', overflow: 'hidden', display: 'grid' }}
/>

See complete code: https://codesandbox.io/s/r1mm947xp

fix(layout): remove truncate* props from Layout and ItemLayout

Closes #849

truncate* props (truncateStart, truncateMain, truncateEnd) failed to handle all truncation cases for Layout (see #849 ) and consequently ItemLayout so we decided to remove them from the API. (BREAKING CHANGE)

@codecov
Copy link

codecov bot commented Mar 28, 2019

Codecov Report

Merging #1127 into master will increase coverage by 0.02%.
The diff coverage is 25%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #1127      +/-   ##
=========================================
+ Coverage   82.37%   82.4%   +0.02%     
=========================================
  Files         733     733              
  Lines        8720    8717       -3     
  Branches     1168    1230      +62     
=========================================
  Hits         7183    7183              
+ Misses       1522    1519       -3     
  Partials       15      15
Impacted Files Coverage Δ
packages/react/src/components/Layout/Layout.tsx 94.64% <ø> (ø) ⬆️
...src/themes/teams/components/Layout/layoutStyles.ts 6.25% <0%> (+0.53%) ⬆️
...ges/react/src/components/ItemLayout/ItemLayout.tsx 82.97% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 71996a4...168fe70. Read the comment docs.

@bmdalex bmdalex force-pushed the fix/layout-remove-truncate-props branch 3 times, most recently from 856ebc9 to 1a81c61 Compare April 2, 2019 09:20
@bmdalex bmdalex force-pushed the fix/layout-remove-truncate-props branch from 251debe to 168fe70 Compare April 2, 2019 11:49
import Rtl from './Rtl'

const ItemLayoutExamples = () => (
<div>
<Alert warning>This component is deprecated</Alert>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's try and be as helpful as we can. Perhaps add link to Layout guide and suggest flex as alternative?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done in #1154

}

/**
* The Item Layout handles layout styles for menu items, list items and other similar item templates.
* (DEPRECATED) The Item Layout handles layout styles for menu items, list items and other similar item templates.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we suggest alternative approach here? Or point to Layout guide?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it should be enough to reference alternative components to use - may suggest not to include any links to the descriptions, as those are subject to potential changes

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done in #1154

@@ -101,8 +99,6 @@ class ItemLayout extends UIComponent<ReactProps<ItemLayoutProps>, any> {
contentCSS: PropTypes.object,
contentMediaCSS: PropTypes.object,
endMediaCSS: PropTypes.object,
truncateContent: PropTypes.bool,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we just take these out, won't they just get passed through to html and not do anything. Should we leave as warning / compile error or something for a few releases before we actually remove?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is already marked as a BREAKING CHANGE in CHANGELOG.md and it points to the description of the PR where we have a mitigation path. This should be enough for our clients to update without too much friction

@kuzhelov kuzhelov merged commit 830d15f into master Apr 2, 2019
@delete-merged-branch delete-merged-branch bot deleted the fix/layout-remove-truncate-props branch April 2, 2019 13:28
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TruncateEnd property in Layout component is not truncating content with ellipsis.
4 participants