Skip to content
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

fix(ellipsis): fix invalid styles in multi-line #546

Merged
merged 1 commit into from Oct 8, 2020
Merged

fix(ellipsis): fix invalid styles in multi-line #546

merged 1 commit into from Oct 8, 2020

Conversation

drakang4
Copy link
Contributor

@drakang4 drakang4 commented Oct 8, 2020

ellipsis in v4 with lines parameter is returning invalid styles.

const Ellipsis = styled.div`
  ${ellipsis("250px", 3)}
`;

const App = () => (
  <Ellipsis>
    Lorem ipsum dolor sit amet, consectetur adipiscing elit. Pellentesque
    accumsan nunc lectus, quis maximus mauris pharetra ut. Nullam cursus
    ullamcorper sem vitae convallis. Pellentesque tempor sit amet nisl id
    dapibus. Curabitur non eleifend velit, ac varius nibh. Etiam leo urna,
    ullamcorper sed turpis a, maximus tristique orci. Quisque eleifend augue
    molestie luctus accumsan. Suspendisse finibus risus vitae ornare
    porttitor. In nisl magna, egestas et blandit ut, imperdiet vel augue.
    Etiam vel risus et leo egestas convallis id vitae lorem. Pellentesque
    maximus elit velit, finibus viverra diam mollis eu. Duis eu dui eget
    dolor tincidunt scelerisque. Phasellus imperdiet scelerisque purus, at
    aliquet erat laoreet et.
  </Ellipsis>
);
Result Styles
image image

The problems and fixes are:

  1. -webkit vendor prefix must be started with uppercase in object notation. Otherwise it turns into just webkit which is incorrect.

  2. When lines > 1, white-space value must be normal. Lines are always considered as 1 if it keeps to nowrap.

Result Styles
image image

@drakang4 drakang4 requested a review from bhough as a code owner October 8, 2020 14:13
@codecov
Copy link

codecov bot commented Oct 8, 2020

Codecov Report

Merging #546 into main will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##              main      #546   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           88        88           
  Lines          831       831           
  Branches       305       305           
=========================================
  Hits           831       831           
Flag Coverage Δ
#unittests 100.00% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/mixins/ellipsis.js 100.00% <ø> (ø)

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 fa58fb2...327103b. Read the comment docs.

@bhough bhough added the bug label Oct 8, 2020
@bhough bhough merged commit 0da1850 into styled-components:main Oct 8, 2020
@bhough
Copy link
Contributor

bhough commented Oct 8, 2020

Thanks for catching this and the quick fix @drakang4. We will cut a new release shortly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants