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

update(Link): Add truncated prop #397

Merged
merged 2 commits into from
Jul 31, 2020
Merged

update(Link): Add truncated prop #397

merged 2 commits into from
Jul 31, 2020

Conversation

elizabeth-moore
Copy link
Contributor

to: @williaster @alecklandgraf

Description

This adds a truncated property to links and a link_truncated style, as well as a unit test. It also removes truncated from the textProps because it doesn't work.

Motivation and Context

Although Link takes in textProps to pass to the Text component, overflow and text-overflow aren't inherited, so the truncation isn't applied. This removes the ability to define truncated in the textProps and adds a prop to Link so that truncating a Link with ellipsis is possible.

Testing

I tested this locally in storyguide and added a unit test.

Screenshots

Screen Shot 2020-07-30 at 4 43 52 PM

Checklist

  • My code follows the style guide of this project.
  • I have updated or added documentation accordingly.
  • I have read the CONTRIBUTING document.

@airbnb-bot
Copy link
Collaborator

Size Changes

Package Diff ESM Prev ESM CJS Prev CJS
core +0.0% 566.97 KB 566.78 KB 709.05 KB 708.85 KB

Compared to master. File sizes are unminified and ungzipped.

View raw build stats

Previous (master)

{
  "apollo": {
    "esm": 10832,
    "lib": 14147
  },
  "app-shell": {
    "esm": 12906,
    "lib": 19874
  },
  "composer": {
    "esm": 68247,
    "lib": 101805
  },
  "core": {
    "esm": 580382,
    "lib": 725859
  },
  "forms": {
    "esm": 37350,
    "lib": 49298
  },
  "icons": {
    "esm": 156355,
    "lib": 205626
  },
  "layouts": {
    "esm": 15298,
    "lib": 20770
  },
  "metrics": {
    "esm": 5467,
    "lib": 7729
  },
  "test-utils": {
    "esm": 4279,
    "lib": 5937
  }
}

Current

{
  "apollo": {
    "esm": 10832,
    "lib": 14147
  },
  "app-shell": {
    "esm": 12906,
    "lib": 19874
  },
  "composer": {
    "esm": 68247,
    "lib": 101805
  },
  "core": {
    "esm": 580573,
    "lib": 726065
  },
  "forms": {
    "esm": 37350,
    "lib": 49298
  },
  "icons": {
    "esm": 156355,
    "lib": 205626
  },
  "layouts": {
    "esm": 15298,
    "lib": 20770
  },
  "metrics": {
    "esm": 5467,
    "lib": 7729
  },
  "test-utils": {
    "esm": 4279,
    "lib": 5937
  }
}

@williaster
Copy link
Contributor

This looks great!

One thought is that a new storybook story might useful since it will give us a happo visual diff test, which will ensure functional correctness more than a unit test which asserts on a className. Happo also seems to be glitching right now (5 diffs for dark only seem unrelated to this), so adding a story and re-running CI might fix that.

@williaster
Copy link
Contributor

All of the jobs passed according to this, but the happo (light) job isn't updating here. gonna merge so we can get this released 🎉

@williaster williaster merged commit 29abe18 into master Jul 31, 2020
@williaster williaster deleted the eam--truncate-link branch July 31, 2020 20:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants