-
Notifications
You must be signed in to change notification settings - Fork 166
[Do-Not-Merge] Add deprecated notice to terra-overlay #3357
Changes from 4 commits
ad102db
9e30f99
2a39e79
abf7a19
1c3802c
fb7d4b8
a63aa91
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,9 @@ | |
|
||
## Unreleased | ||
|
||
* Added | ||
* Added a deprecated notice. | ||
|
||
## 3.62.0 - (February 2, 2021) | ||
|
||
* Changed | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
|
||
A Loading Overlay is a subcomponent that displays an overlay with a spinner icon and loading message. | ||
The terra-overlay npm package is deprecated as of 3.62.0. terra-overlay will be removed from the repo in a future version, but the package will remain on npm. | ||
pranav300 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
### What deprecation means | ||
Any apps that currently use terra-overlay will continue to work as-is. | ||
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 commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more. Updated here a63aa91, with what @tbiethman mentioned ☝️. |
||
|
||
## How to replace this package? | ||
This package can be replaced with [terra-application-loading-overlay](https://engineering.cerner.com/terra-ui/application/terra-application/components/application-loading-overlay) | ||
|
||
~~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. | ||
|
||
A Loading Overlay is a subcomponent that displays an overlay with a spinner icon and loading message.~~ | ||
|
||
- [Getting Started](#getting-started) | ||
- [Documentation](https://engineering.cerner.com/terra-ui/components/terra-overlay/overlay/overlay) | ||
|
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