LPS-113552 Deprecated and replaced Liferay.Alert usages with Liferay.Util.openToast #90413
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ed on openToast for open the toast relatively to an element
…til.openToast` utility
… Clay docs and point up the complete docs
…he children on a fragment
…ct to render Also, wraps the AlertContainer into a Dynamic Component when having containerId or container
Better container and containerId descriptions
I was originally going to fix the grammar but I don't think the comment is worth keeping.
Prior to the recent refactoring, the code in this module used to have
this behavior:
- If no container ID is provided, we create and use a default container,
injecting it into the body.
- Subsequent toasts without container IDs will reuse the same default
container.
- If we then provide a container (or container ID), the default
container is ignored.
Due to refactoring, the behavior changed to:
- A div with the default container ID is always unconditionally created
(bad).
- If no container ID is provided, we use the default container,
injecting it into the body.
- Subsequent toasts without a container ID will inject additional copies
if the container (bad), with identical IDs (bad).
- If we then provide a container (or container ID), we append yet
another copy of the default container as a child, again with an
identical ID (bad).
So, this commit fixes that:
- Default container is only created as a fallback (ie. when no valid
container or container ID is provided).
- Default container is re-used if fallback is needed multiple times (ie.
no duplicate ID in DOM).
It preserves the change which originally motivated the refactoring:
- If a container is provided, a div is inserted into it, into which to
mount the toasts (this is so that React won't blow away the entire
contents of the container).
A suboptimal, but not the end of the world, consequence of this is that,
because these inserted divs don't have IDs, we don't have any way of
reusing them if multiple toasts are shown, so they will accumulate in
the DOM until the next navigation or other update removes their parents.
I renamed the `getContainerElement` function to `getRootElement` to
reduce the overloading of the word "container" just a little bit: React
uses the word "container" to describe the node into which
ReactDOM.render mounts a compenent, and Clay uses the term
`ToastContainer` for the component that contains the toast. The naming
still sucks, but at least now we can write:
const rootElement = getRootElement(...);
|
To conserve resources, the PR Tester does not automatically run for forwarded pull requests. |
|
Merged. Thank you. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Forwarded from: liferay-frontend#24 (Took 1
ci:forwardattempt in 1 minute)@wincent
@liferay-frontend
Original pull request comment:
Rebase of: liferay-frontend#7
To resolve conflicts with: #90226
And to fix bugs; specifically, see: liferay-frontend@793a105
cc @diegonvs
Original message follows.
Originally: https://github.com/wincent/liferay-portal/pull/334
Forwarding this here to keep everything under liferay-frontend user.
cc @diegonvs
Original message follows.
AUI liferay-alert Deprecation
When rendering the component, running:
I noticed that styles of Alert are very similar to openToast:
However, liferay-alert has wrong styles. So, a migration for using
openToastwill be nicer for improving not just the style but deprecating an old AUI component.Finding the usages “grepping”:
git grep '\bLiferay\.Alert\b'git grep liferay-alertliferay-logo-editorAUI component is currently usingliferay-alertand was replaced withopenToast. This alert don’t have a title, by default theopenToastcommand adds aSuccesstitle. This behaviour was changed here: LPS-113582|LPS-113583 Replaces the usages ofLiferay.NotificationandLiferay.NoticewithLiferay.Util.openToastby diegonvs · Pull Request #309 · wincent/liferay-portal · GitHubAPI Usages:
/I noticed for some examples, an element or a selector is being passed to the component (via render function) and the toast will open near this element.
Considering this, I’m using the openToast like:
Or
Or
liferay-notificationAUI Plugin also depends onliferay-alert. However this plugin was deprecated at https://github.com/wincent/liferay-portal/pull/309.ImageEditorcomponent was usingliferay-alertdynamic requiring the module usingAUI().use. The usage can be easily replaced withopenToast.On
journal_template.jsp, the plugin was easily replaced. The alert element inserted to the templatePreview doesn’t have the function of Liferay.Alert and don’t depend on it.On
add_layout.jsp, it was very easy for change to replace and I also changed the script tag fromaui:scripttoscript. Sinceliferay-alertdependency is not being used more.On
add_site_navigation_menu_item.jsp, I conducted the same as the JSP above.On
portal-workflow-web, it was quickly replaced byLiferay.Util.openToast.On
calendar-web, I noticed that they gives a container to the Alert component. e.ginstance.get(‘rootNode). I solved this usinginstance.get('rootNode')._nodethat returns the corresponding DOM element of the Y.Node. Also,showAlertwas changed fromY.Alertto openToast.Questions:
There are some cases using data-dismiss=“liferay-alert”. Is this alert support depending on
liferay-alertoraui-alert? cc @markocikos✔️ ci:test:sf - 1 out of 1 jobs passed in 3 minutes
Click here for more details.
Base Branch:
Branch Name: master
Branch GIT ID: 0a1ebcca26d3eabf9e443f7aa371b20d9f85ccd5
Sender Branch:
Branch Name: LPS-113552
1 out of 1jobs PASSEDBranch GIT ID: 424aaf71d3db1b989495b7a9c96ad9caf2c240e4
1 Successful Jobs:
For more details click here.
✔️ ci:test:stable - 20 out of 20 jobs passed
✔️ ci:test:relevant - 92 out of 92 jobs passed in 1 hour 29 minutes
Click here for more details.
Base Branch:
Branch Name: master
Branch GIT ID: 0a1ebcca26d3eabf9e443f7aa371b20d9f85ccd5
Copied in Private Modules Branch:
Branch Name: master-private
Branch GIT ID: 1eb935eac6911217dfe48eb2379d11fb23dd7f33
ci:test:stable - 20 out of 20 jobs PASSED
20 Successful Jobs:
ci:test:relevant - 92 out of 92 jobs PASSED
92 Successful Jobs:
For more details click here.