-
Notifications
You must be signed in to change notification settings - Fork 166
[Do-Not-Merge] Add deprecated notice to terra-overlay #3357
Conversation
@@ -4,9 +4,23 @@ | |||
[](https://www.npmjs.org/package/terra-overlay) | |||
[](https://travis-ci.com/cerner/terra-core) | |||
|
|||
The Terra Overlay component is a component that displays an overlay relative to the container that triggered the overlay. This component blocks interactions and fades out all elements of the triggering container. | |||
## Deprecation Notice |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if we converted this into mdx, we could use our deprecation notice component: https://engineering.cerner.com/terra-ui/dev_tools/cerner/documentation-components/notice#notices-for-implementation-suggestions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We shouldn't convert the readme to mdx. It needs to be renderable by github.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and npm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oops good call, didn't realize this was the readme
For any of these depreciation notices we need to do the following.
---After PR----
|
packages/terra-overlay/README.md
Outdated
However, the terra-overlay package will no longer continue to be supported / developed. | ||
|
||
## Why? | ||
Terra Overlay design is a bad pattern and hence it is being deprecated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you mean by this is a bad pattern? being devils advocate, without any background context, I don't really feel like this sells me on why it should deprecated.
Also what is different about terra-application-loading-overlay
that make it a better pattern?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, i was going to get to that once we got some other things cleaned up, but it's not a bad pattern it's just an old pattern and now we want people to pull from terra-application instead. There will have to be some more tweaking of the wording here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd say the biggest reason for this deprecation is to integrate the presentation of overlays with the overall application framework to ensure accessibility. Improving the ease of use of these components is a plus, too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated here a63aa91, with what @tbiethman mentioned ☝️.
Co-authored-by: Emily Rohrbough <[email protected]>
Closing this PR. Due to delay in terra-application v2 release, marking these components deprecated seems premature. The deprecations of this will be done once terra-application v2 is released. |
Summary
Added a deprecated notice to terra-overlay
The plan is to wait till terra-application v2 is released and then update this PR with correct links for migration guide.
Till then this can work as a template for deprecation and maintenance PR's.
Closes #3331
Thank you for contributing to Terra.
@cerner/terra