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

Better breadcrumb truncation #1346

Merged
merged 7 commits into from
Dec 6, 2018
Merged

Conversation

cchaos
Copy link
Contributor

@cchaos cchaos commented Dec 5, 2018

Before this PR, applying truncation to EuiBreadcrumbs would max-width every breadcrumb to 160px. This caused some issues with the last item being unnecessarily truncated.

In this PR, I've altered the way EuiBreadcrumbs handle the truncate prop as well as added the ability to add truncate to individual breadcrumbs.

The new behavior is this:

  1. Applying truncate to an individual breadcrumb (in the breadcrumbs object prop) will singly max-width that breadcrumb at 160px. This allows for more granular handling of the truncation ability.
  2. Applying truncate to the whole EuiBreadcrumbs component, will contain the whole thing to a single line of text and max-width every breadcrumb at 160px except for the last item. This last item will still truncate if it meets the right side of the container.

image

EuiHeader

In order for these to truncate properly, the breadcrumb container had to be a direct child of the EuiHeader. This means, that the breadcrumbs need to be pulled out of a EuiHeaderSection. This meant I had to alter/add some of the props for those items as well.

Example of it working

https://d.pr/free/v/6Ge7ZD

Checklist

  • This was checked in mobile
  • This was checked in IE11
  • This was checked in dark mode
  • Any props added have proper autodocs
  • Documentation examples were added
  • A changelog entry exists and is marked appropriately
  • This was checked for breaking changes and labeled appropriately
  • Jest tests were updated or added to match the most common scenarios
  • [ ] This was checked against keyboard-only and screenreader scenarios
  • [ ] This required updates to Framer X components

Copy link
Contributor

@snide snide left a comment

Choose a reason for hiding this comment

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

Tested in IE11 for ya..... and it works fine :)

Left some small comments on the docs. This is a clever solution and works much better than the original.

image

src/components/breadcrumbs/breadcrumbs.js Show resolved Hide resolved
src-docs/src/views/breadcrumbs/truncate.js Show resolved Hide resolved
@cchaos
Copy link
Contributor Author

cchaos commented Dec 5, 2018

Awesome, thank you for testing in IE @snide!!

@snide
Copy link
Contributor

snide commented Dec 5, 2018 via email

@cchaos
Copy link
Contributor Author

cchaos commented Dec 5, 2018

Sounds good! Thanks!

Copy link
Contributor

@spalger spalger left a comment

Choose a reason for hiding this comment

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

Looks awesome and works great in the docs, doesn't work in Kibana though. Probably not a blocker for merging this PR:

2018-12-05 21 20 56

@cchaos
Copy link
Contributor Author

cchaos commented Dec 6, 2018

@spalger, Sorry I wasn't too clear on the changes that need to be made in the EuiHeader. But in order for this to work properly, the EuiHeaderBreadcrumbs needs to be a direct descendant of EuiHeader and not inside of a EuiHeaderSection. If you alter the implementation like so:

image

Then it will behave like so:

erg, something's broke with github's embedded img's, here's the link: https://d.pr/free/i/zplxwe

@cchaos cchaos merged commit c2ad62e into elastic:master Dec 6, 2018
@cchaos cchaos deleted the breadcrumb-truncation branch December 6, 2018 23:08
@snide snide mentioned this pull request Dec 7, 2018
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants