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

Deprecations, October 2021 Edition #5323

Merged
merged 27 commits into from
Oct 29, 2021

Conversation

thompsongl
Copy link
Contributor

@thompsongl thompsongl commented Oct 26, 2021

Summary

Completes the October 2021 round of deprecations (#1469)

Best (re)viewed by individual commit

Upgrade path: https://gist.github.com/thompsongl/506f6f86201c83d61cc850170fd49467

Checklist

- [ ] Check against all themes for compatibility in both light and dark modes
- [ ] Checked in mobile
- [ ] Checked in Chrome, Safari, Edge, and Firefox

- [ ] Added documentation

  • Checked Code Sandbox works for any docs examples
  • Added or updated jest and cypress tests
  • Checked for breaking changes and labeled appropriately

- [ ] Checked for accessibility including keyboard-only and screenreader modes

  • A changelog entry exists and is marked appropriately

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_5323/

@thompsongl
Copy link
Contributor Author

Opening for review but won't merge prior to the October 26ish release (v40.1.0).

@thompsongl thompsongl marked this pull request as ready for review October 26, 2021 19:18
@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_5323/

@thompsongl
Copy link
Contributor Author

jenkins test this

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_5323/

@@ -71,7 +71,6 @@
"mdast-util-to-hast": "^10.0.0",
"numeral": "^2.0.6",
"prop-types": "^15.6.0",
"react-ace": "^7.0.5",
Copy link
Contributor

Choose a reason for hiding this comment

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

Unable to describe the joy I feel

Copy link
Contributor

@chandlerprall chandlerprall left a comment

Choose a reason for hiding this comment

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

Changes LGTM, lots of great deletions 🎉

Curious if you've tried a build with these in Kibana, or otherwise have an idea on the effort this update will require?

@thompsongl
Copy link
Contributor Author

Curious if you've tried a build with these in Kibana, or otherwise have an idea on the effort this update will require?

Planning to post a comment about the upgrade path today. secondary -> success is going to be tedious, but I'll get some numbers behind it.

@thompsongl
Copy link
Contributor Author

@thompsongl
Copy link
Contributor Author

Some stats from Kibana:

  • 81 direct uses of color="secondary" but additional interpolated cases exist color={buttonColor}
    • type_check will be helpful
    • Sifting through js files for secondary strings is going to be rough 🥵
  • A few instances of color="subdued" on EuiButtonIcon
    • type_check will be helpful
    • Note that other components (e.g., EuiText, EuiPanel) don't need updating
  • Quite a few mobile option props
    • hideForMobile, mainly, but these are easy to search for
    • fullWidth will require some sifting though search results
  • 9 instances of EuiLoadingKibana
  • 6 instances of EuiCodeEditor imported from @elastic/eui
  • 0 instances of makeId 🎉

Copy link
Contributor

@cchaos cchaos left a comment

Choose a reason for hiding this comment

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

I'm going to test this incrementally per commit as suggested.

✅ EuiCard beta badge

I'm pushing up a few more updates mainly to the docs and prop comment.

src/components/card/card.tsx Show resolved Hide resolved
- Convert touched docs files to `.tsx`
- Simplified one of the examples
- Updated prop comment
Copy link
Contributor

@cchaos cchaos left a comment

Choose a reason for hiding this comment

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

✅ [EuiPageContent] panelPaddingSize

Copy link
Contributor

@cchaos cchaos left a comment

Choose a reason for hiding this comment

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

✅ EuiButtonIcon subdued

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_5323/

@thompsongl
Copy link
Contributor Author

This will have an even greater impact, so I'm ok if we want to do a second PR specifically for theme tokens.

Great, let's do a second PR for that

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_5323/

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_5323/

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_5323/

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_5323/

cchaos added 4 commits October 28, 2021 14:03
And fixed `align: center` for mobile
- [EuiMark] change prop type of `children` from `string` to `ReactNode`
- [useEuiTextDiff] renders back a `span` instead of `div`
- Fix docs-only erros
@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_5323/

Copy link
Contributor

@cchaos cchaos left a comment

Choose a reason for hiding this comment

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

😅 Whew! Can't wait for the Kibana upgrade ... lol.

I did a final run though of our docs pages checking for console errors. We should be good to go!

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_5323/

@thompsongl
Copy link
Contributor Author

Thanks for the assist, @cchaos

Merging 🙈

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.

4 participants