Update terra-base and terra-i18n packages #2288
Conversation
…, and releated jest tests
bjankord
requested review from
ryanthemanuel,
mjhenkes,
tbiethman and
JakeLaCombe
bjankord
force-pushed
the
update-base-and-i18n-packages
branch
from
9798777 to
b63c469
Compare
bjankord
force-pushed
the
update-base-and-i18n-packages
branch
from
1b43992 to
b63c469
Compare
| @@ -46,7 +46,7 @@ const defaultProps = { | |||
| isOpen: false, | |||
| backgroundStyle: BackgroundStyles.LIGHT, | |||
| isRelativeToContainer: false, | |||
| rootSelector: '[data-terra-base]', | |||
| rootSelector: '#root', | |||
There was a problem hiding this comment.
It seems we're making an assumption that the there is a root selector present. That assumption works for terra-dev-site and most orion apps, but it's not guaranteed that apps will have the root selector. If we want to change this from the base selector we might want to open it up to a prop but default to root and document it on the readme.
There was a problem hiding this comment.
We have a prop for this already named rootSelector. This line sets the default value based on that assumption that there is an id="root" in the DOM. For apps that don't have that id attribute, they can change it by passing a different selector to the rootSelector prop. I can update the prop type documentation to be more clear about this.
There was a problem hiding this comment.
If we already have a prop for this i don't think you have to document more. I should have looked more deeply for the doc.
There was a problem hiding this comment.
Updated the doc, the doc referenced a data attribute that was on terra-base div that used to be rendered. We've removed that div to fix terra-base fill issues, but with that, we needed to update the rootSelector we default to. c7e816f
Summary
BREAKING CHANGES