LPS-116254 Localization - alert success notifications are not displayed correctly #78

diegonvs · 2020-07-03T21:47:57Z

Test Case:

Open Blogs
Change the language to another language replacing the URL before /group adding for example pt e.g(http://localhost:8080/pt/group/guest/~/control_panel/manage?p_p_id=com_liferay_blogs_web_portlet_BlogsAdminPortlet&p_p_lifecycle=0&p_p_state=maximized&p_p_mode=view&p_p_auth=UJfDKe6X)
Click to add a new blog entry
Fill the blog post with required things and then publish

Expected results:

No JS errors on console and the toast message (Your request was finished) will be shown adequately.

What I did

I noticed other toast containers created by JSP for example can be conflicting with React renderization of openToast. So, I decided to replace the DEFAULT_CONTAINER_ID to a unique ID used on the openToast, for not conflicting with others.

Questions

However the result was:

We aren't able to stack toast Alerts with this current implementation.

One possible thing to fix this issue is creating another command like openAlert for isolate the logic for containerId oriented alerts and left the openToast behaviour like the past API(Where We could just stack toasts and We aren't using it for every alert We got).

What do you think about it folks?

/cc @wincent @jbalsas @matuzalemsteles @markocikos @brunobasto

liferay-continuous-integration · 2020-07-03T21:48:05Z

To conserve resources, the PR Tester does not automatically run for every pull.

If your code changes were already tested in another pull, reference that pull in this pull so the test results can be analyzed.

If your pull was never tested, comment "ci:test" to run the PR Tester for this pull.

diegonvs · 2020-07-03T21:48:48Z

ci:test:sf

liferay-continuous-integration · 2020-07-03T22:03:19Z

✔️ ci:test:sf - 1 out of 1 jobs passed in 14 minutes

Click here for more details.

Base Branch:

Branch Name: master
Branch GIT ID: a31b0cd4a88060b1100c38cb37dd00fe6bf1a737

Sender Branch:

Branch Name: LPS-116254
Branch GIT ID: b44f22156a804d261e7623a4331376416bbeb0b9

1 out of 1jobs PASSED

1 Successful Jobs:

test-portal-source-format

For more details click here.

liferay-continuous-integration · 2020-07-03T22:05:11Z

Jenkins Build:test-portal-source-format#2947
Jenkins Report:jenkins-report.html
Jenkins Suite: sf
Pull Request:liferay-frontend#78
Spira Release:/Liferay DXP 7.3/Pull Request/ci:test:sf
Spira Release Build:master - diegonvs > liferay-frontend - PR#78
Spira Jenkins Build:publish-spira-report#85

diegonvs · 2020-07-03T22:06:52Z

ci:test:relevant

liferay-continuous-integration · 2020-07-03T23:25:47Z

✔️ ci:test:stable - 20 out of 20 jobs passed

✔️ ci:test:relevant - 50 out of 50 jobs passed in 1 hour 18 minutes

Click here for more details.

Base Branch:

Branch Name: master
Branch GIT ID: a31b0cd4a88060b1100c38cb37dd00fe6bf1a737

Copied in Private Modules Branch:

Branch Name: master-private
Branch GIT ID: 26d9c30572aa163b37c2bdb9a90c3ef3cbf122b3

ci:test:stable - 20 out of 20 jobs PASSED

20 Successful Jobs:

ci:test:relevant - 50 out of 50 jobs PASSED

50 Successful Jobs:

For more details click here.

liferay-continuous-integration · 2020-07-03T23:27:48Z

Jenkins Build:test-portal-acceptance-pullrequest(master)#7732
Jenkins Report:jenkins-report.html
Jenkins Suite: relevant
Pull Request:liferay-frontend#78
Spira Release:/Liferay DXP 7.3/Pull Request/ci:test:relevant
Spira Release Build:master - diegonvs > liferay-frontend - PR#78
Spira Jenkins Build:publish-spira-report#88

wincent · 2020-07-06T13:48:41Z

We aren't able to stack toast Alerts with this current implementation.

One possible thing to fix this issue is creating another command like openAlert for isolate the logic for containerId oriented alerts and left the openToast behaviour like the past API(Where We could just stack toasts and We aren't using it for every alert We got).

Sounds like we should just change openToast() to stack by default. If we had openAlert() with stacking and openToast() without (where last-written message overwrites previous), I just wouldn't use openToast() at all, because the existing behavior just seems broken.

I looked to see if we had any Lexicon guidance on how toasts should behave in the presence of multiple simultaneous ones, but couldn't find anything here), but this sounds to me like something that would be desirable for it to define. cc @drakonux

Separate topic is your fix here of changing the ID:

I noticed other toast containers created by JSP for example can be conflicting with React renderization of openToast. So, I decided to replace the DEFAULT_CONTAINER_ID to a unique ID used on the openToast, for not conflicting

Given that the number of these other call sites is small (2), I think it would make sense for them to be somehow made compatible. If we do implement stacking in openToast(), then it doesn't seem good if we have two separate containers on the page that aren't participating in a coordinated way in the stacking mechanism.

At least in util-taglib, we could just depend on React directly, because it is an OSGi module. The portal-web/docroot thing would require us to be a little more creative, but we could still probably figure something out.

matuzalemsteles · 2020-07-06T14:16:24Z

We aren't able to stack toast Alerts with this current implementation.

One possible thing to fix this issue is creating another command like openAlert for isolate the logic for containerId oriented alerts and left the openToast behaviour like the past API(Where We could just stack toasts and We aren't using it for every alert We got).

Ideally, Toasts should stack, we should trigger Toasts inside <ClayAlert.ToastContainer /> so that it can stack, so I see two ways to do with this current implementation:

We removed the <ClayAlert.ToastContainer /> from the component and manually created the container for default implementations without customizing a container id or container. In the same way that we are doing manually.
We try to make the Component instance persist, rendering only once and the next calls of openToast only cause a rendering and update the state. As in the example: https://clayui.com/docs/components/alert.html#toastcontainer

I believe that the second option could generate more complications than that, so maybe the first option would be easier ?!

wincent · 2020-07-07T17:03:55Z

We removed the <ClayAlert.ToastContainer /> from the component and manually created the container for default implementations without customizing a container id or container. In the same way that we are doing manually.

Sounds good. @diegonvs, do you want to take a shot at refactoring this to get <ClayAlert.ToastContainer /> in place for all cases. Seems reasonable, seeing as it used to always apply before (ie. prior to 232f6f0439430)? Do you need any help?

diegonvs · 2020-07-07T17:25:32Z

do you want to take a shot at refactoring this to get <ClayAlert.ToastContainer /> in place for all cases

I can do. If I catch any limitation I can post here.

diegonvs · 2020-07-08T00:24:50Z

Updated. Now, ToastContainer is created manually when not receiving container or containerId and ClayAlert React controlled components are being appended there.

Now We're able to stack(when calling multiple times) using openToast when:

passing containerId ✅
passing container ✅
passing containerId ✅

I used metal-dom's buildFragment function for creating html from string, instead of declaratively and verbosely calling createElement etc calls.

Just for curiosity:

Should We avoid using metal-dom utilities?
If no, Do we have plans to migrate these utilities into our DXP codebase?

diegonvs · 2020-07-08T00:25:03Z

ci:test:relevant

diegonvs · 2020-07-08T00:25:08Z

ci:test:sf

liferay-continuous-integration · 2020-07-20T22:10:15Z

✔️ ci:test:stable - 20 out of 20 jobs passed

❌ ci:test:relevant - 50 out of 52 jobs passed in 1 hour 19 minutes

Click here for more details.

Base Branch:

Branch Name: master
Branch GIT ID: 8603a4438fe82f3c80f21ff3cb83648f00e78b62

Copied in Private Modules Branch:

Branch Name: master-private
Branch GIT ID: 9df5a717f14278f29e7460d7055a941d93e0b735

ci:test:stable - 20 out of 20 jobs PASSED

20 Successful Jobs:

ci:test:relevant - 50 out of 52 jobs PASSED

2 Failed Jobs:

50 Successful Jobs:

For more details click here.

Failures unique to this pull:

test-portal-acceptance-pullrequest-batch(master)/functional-tomcat90-mysql57-jdk8/7
Job Results:
1 Test Passed.
1 Test Failed.
1. AXIS_VARIABLE=0,label_exp=!master #390079
  1. LocalFile.LCSClientSmoke#LCSClientSmoke - Poshi Report - Poshi Summary - Console Output

For upstream results, click here.

liferay-continuous-integration · 2020-07-20T22:12:33Z

Jenkins Build:test-portal-acceptance-pullrequest(master)#6953
Jenkins Report:jenkins-report.html
Jenkins Suite: relevant
Pull Request:liferay-frontend#78
Spira Release:/Liferay DXP 7.3/Pull Request/ci:test:relevant
Spira Release Build:master - diegonvs > liferay-frontend - PR#78 - 2020-07-20[13:51:07]
Spira Jenkins Build:publish-spira-report#980

john-co · 2020-07-20T22:30:48Z

Reviewed. Test failures are unrelated. ✔️
This can be manually forwarded.

Updating our ci:test:relevant filter in diegonvs#34. I don't have push rights to @diegonvs/liferay-portal.

diegonvs · 2020-07-21T02:13:09Z

Hey @john-co, I invited you

john-co · 2020-07-21T19:30:41Z

ci:test:relevant

liferay-continuous-integration · 2020-07-22T00:30:55Z

✔️ ci:test:stable - 20 out of 21 jobs passed

✔️ ci:test:relevant - 50 out of 51 jobs passed in 2 hours 11 minutes

Click here for more details.

Base Branch:

Branch Name: master
Branch GIT ID: 21b2adc009b50747fa2512a77308e78001e6b13f

Copied in Private Modules Branch:

Branch Name: master-private
Branch GIT ID: 9df5a717f14278f29e7460d7055a941d93e0b735

ci:test:stable - 20 out of 21 jobs PASSED

1 Failed Jobs:

~~test-portal-acceptance-pullrequest-batch(master)/modules-unit-project-templates-jdk8~~

20 Successful Jobs:

ci:test:relevant - 49 out of 51 jobs PASSED

2 Failed Jobs:

49 Successful Jobs:

For more details click here.

This pull contains no unique failures.

Failures in common with acceptance upstream results at 21b2adc:

test-portal-acceptance-pullrequest-batch(master)/modules-unit-project-templates-jdk8

Job Results:

157 Tests Passed.
20 Tests Failed.

AXIS_VARIABLE=0,label_exp=!master #418985

ProjectTemplatesFormFieldTest.testBuildTemplateFormField72Maven

java.lang.AssertionError: [INFO] Error stacktraces are turned on.
[INFO] Scanning for projects...
[INFO] Downloading: https://repository-cdn.liferay.com/nexus/content/groups/public/com/liferay/portal/release.portal.bom/7.2.1/release.portal.bom-7.2.1.pom
[INFO] Downloaded: https://repository-cdn.liferay.com/nexus/content/groups/public/com/liferay/portal/release.portal.bom/7.2.1/release.portal.bom-7.2.1.pom (0 B at 0 B/s)
[INFO] Downloading: https://repository-cdn.liferay.com/nexus/content/groups/public/com/liferay/portal/release.portal.bom.compile.only/7.2.1/release.portal.bom.compile.only-7.2.1.pom
[INFO] Downloaded: https://repository-cdn.liferay.com/nexus/content/groups/public/com/liferay/portal/release.portal.bom.compile.only/7.2.1/release.portal.bom.compile.only-7.2.1.pom (0 B at 0 B/s)
[INFO] Downloading: https://repository-cdn.liferay.com/nexus/content/groups/public/com/liferay/portal/release.portal.bom.third.party/7.2.1/release.portal.bom.third.party-7.2.1.pom
[INFO] Downloaded: https://repository-cdn.liferay.com/nexus/content/groups/public/com/liferay/portal/release.portal.bom.third.party/7.2.1/release.portal.bom.third.party-7.2.1.pom (0 B at 0 B/s)
[INFO] ------------------------------------------------------------------------
[INFO] Reactor Build Order:
[INFO] 
[INFO] mavenWS
[INFO] mavenWS Modules
[INFO] mavenWS Themes
[INFO] Downloading: https://repository-cdn.liferay.com/nexus/content/groups/public/com/liferay/com.liferay.portal.tools.bundle.support/3.6.0/com....

ProjectTemplatesFormFieldTest.testBuildTemplateFormField70

java.lang.AssertionError: [INFO] Error stacktraces are turned on.
[INFO] Scanning for projects...
[INFO] Downloading: https://repository-cdn.liferay.com/nexus/content/groups/public/com/liferay/portal/release.portal.bom/7.0.6-1/release.portal.bom-7.0.6-1.pom
[INFO] Downloaded: https://repository-cdn.liferay.com/nexus/content/groups/public/com/liferay/portal/release.portal.bom/7.0.6-1/release.portal.bom-7.0.6-1.pom (0 B at 0 B/s)
[INFO] Downloading: https://repository-cdn.liferay.com/nexus/content/groups/public/com/liferay/portal/release.portal.bom.compile.only/7.0.6-1/release.portal.bom.compile.only-7.0.6-1.pom
[INFO] Downloaded: https://repository-cdn.liferay.com/nexus/content/groups/public/com/liferay/portal/release.portal.bom.compile.only/7.0.6-1/release.portal.bom.compile.only-7.0.6-1.pom (0 B at 0 B/s)
[INFO] Downloading: https://repository-cdn.liferay.com/nexus/content/groups/public/com/liferay/portal/release.portal.bom.third.party/7.0.6-1/release.portal.bom.third.party-7.0.6-1.pom
[INFO] Downloaded: https://repository-cdn.liferay.com/nexus/content/groups/public/com/liferay/portal/release.portal.bom.third.party/7.0.6-1/release.portal.bom.third.party-7.0.6-1.pom (0 B at 0 B/s)
[INFO] Downloading: https://repository-cdn.liferay.com/nexus/content/groups/public/com/liferay/com.liferay.portal.tools.bundle.support/3.6.0/com.liferay.portal.tools.bundle.support-3.6.0.pom
[INFO] Downloaded: https://repository-cdn.liferay.com/nexus/content/groups/public/com/liferay/com.liferay....

ProjectTemplatesFormFieldTest.testBuildTemplateFormField71

java.lang.AssertionError: [INFO] Error stacktraces are turned on.
[INFO] Scanning for projects...
[INFO] Downloading: https://repository-cdn.liferay.com/nexus/content/groups/public/com/liferay/portal/release.portal.bom/7.1.3/release.portal.bom-7.1.3.pom
[INFO] Downloaded: https://repository-cdn.liferay.com/nexus/content/groups/public/com/liferay/portal/release.portal.bom/7.1.3/release.portal.bom-7.1.3.pom (0 B at 0 B/s)
[INFO] Downloading: https://repository-cdn.liferay.com/nexus/content/groups/public/com/liferay/portal/release.portal.bom.compile.only/7.1.3/release.portal.bom.compile.only-7.1.3.pom
[INFO] Downloaded: https://repository-cdn.liferay.com/nexus/content/groups/public/com/liferay/portal/release.portal.bom.compile.only/7.1.3/release.portal.bom.compile.only-7.1.3.pom (0 B at 0 B/s)
[INFO] Downloading: https://repository-cdn.liferay.com/nexus/content/groups/public/com/liferay/portal/release.portal.bom.third.party/7.1.3/release.portal.bom.third.party-7.1.3.pom
[INFO] Downloaded: https://repository-cdn.liferay.com/nexus/content/groups/public/com/liferay/portal/release.portal.bom.third.party/7.1.3/release.portal.bom.third.party-7.1.3.pom (0 B at 0 B/s)
[INFO] Downloading: https://repository-cdn.liferay.com/nexus/content/groups/public/com/liferay/com.liferay.portal.tools.bundle.support/3.6.0/com.liferay.portal.tools.bundle.support-3.6.0.pom
[INFO] Downloaded: https://repository-cdn.liferay.com/nexus/content/groups/public/com/liferay/com.liferay.portal.tools.bundle.supp...

...

liferay-continuous-integration · 2020-07-22T00:34:23Z

Jenkins Build:test-portal-acceptance-pullrequest(master)#7885
Jenkins Report:jenkins-report.html
Jenkins Suite: relevant
Pull Request:liferay-frontend#78
Spira Release:/Liferay DXP 7.3/Pull Request/ci:test:relevant
Spira Release Build:master - diegonvs > liferay-frontend - PR#78 - 2020-07-21[15:19:14]
Spira Jenkins Build:publish-spira-report#1126

brunobasto · 2020-07-22T14:44:51Z

Hey @diegonvs, I noticed @wincent added the needs update on this one. I think that means we need to apply the feedback before forwarding, right?

diegonvs · 2020-07-22T14:56:41Z

Hey @brunobasto, I don't need to update this for now. See: #78 (comment), #78 (comment)

brunobasto · 2020-07-22T15:06:11Z

Oh thanks. Feel free to forward it then :)

diegonvs · 2020-07-22T15:56:36Z

ci:forward

liferay-continuous-integration · 2020-07-22T15:56:40Z

CI is automatically triggering the following test suites:

ci:test:relevant
ci:test:sf

The pull request will automatically be forwarded to the user brianchandotcom if the following test suites pass:

ci:test:relevant
ci:test:sf
ci:test:stable

liferay-continuous-integration · 2020-07-22T15:56:41Z

Skipping previously passed test suites:
ci:test:relevant
ci:test:sf

liferay-continuous-integration · 2020-07-22T15:58:01Z

All required test suite(s) passed.
Forwarding pullrequest to brianchandotcom.

liferay-continuous-integration · 2020-07-22T15:58:30Z

Pull request has been successfully forwarded to brianchandotcom#91652

diegonvs requested review from brunobasto, jbalsas, markocikos, matuzalemsteles and wincent July 3, 2020 21:49

liferay-continuous-integration added ci:test:sf - pending ci:test:sf - success and removed ci:test:sf - pending labels Jul 3, 2020

liferay-continuous-integration added ci:test:relevant - pending ci:test:stable - pending ci:test:relevant - success ci:test:stable - success and removed ci:test:relevant - pending ci:test:stable - pending labels Jul 3, 2020

wincent assigned diegonvs Jul 7, 2020

wincent added the needs update label Jul 7, 2020

liferay-continuous-integration added ci:test:stable - pending ci:test:relevant - failure ci:test:stable - success and removed ci:test:relevant - pending ci:test:stable - pending labels Jul 20, 2020

liferay-continuous-integration added ci:test:relevant - pending ci:test:stable - pending ci:test:relevant - success ci:test:stable - success and removed ci:test:relevant - failure ci:test:stable - success ci:test:relevant - pending ci:test:stable - pending labels Jul 21, 2020

liferay-continuous-integration mentioned this pull request Jul 22, 2020

LPS-116254 Localization - alert success notifications are not displayed correctly brianchandotcom/liferay-portal#91652

Closed

liferay-continuous-integration closed this Jul 22, 2020

LPS-116254 Localization - alert success notifications are not displayed correctly #78

LPS-116254 Localization - alert success notifications are not displayed correctly #78

Conversation

diegonvs commented Jul 3, 2020

Test Case:

What I did

Questions

liferay-continuous-integration commented Jul 3, 2020

diegonvs commented Jul 3, 2020

liferay-continuous-integration commented Jul 3, 2020

✔️ ci:test:sf - 1 out of 1 jobs passed in 14 minutes

Base Branch:

Sender Branch:

For more details click here.

liferay-continuous-integration commented Jul 3, 2020

diegonvs commented Jul 3, 2020

liferay-continuous-integration commented Jul 3, 2020

✔️ ci:test:stable - 20 out of 20 jobs passed

✔️ ci:test:relevant - 50 out of 50 jobs passed in 1 hour 18 minutes

Base Branch:

Copied in Private Modules Branch:

For more details click here.

liferay-continuous-integration commented Jul 3, 2020

wincent commented Jul 6, 2020 • edited

matuzalemsteles commented Jul 6, 2020

wincent commented Jul 7, 2020

diegonvs commented Jul 7, 2020

diegonvs commented Jul 8, 2020

diegonvs commented Jul 8, 2020

diegonvs commented Jul 8, 2020

liferay-continuous-integration commented Jul 20, 2020

✔️ ci:test:stable - 20 out of 20 jobs passed

❌ ci:test:relevant - 50 out of 52 jobs passed in 1 hour 19 minutes

Base Branch:

Copied in Private Modules Branch:

2 Failed Jobs:

For more details click here.

Failures unique to this pull:

test-portal-acceptance-pullrequest-batch(master)/functional-tomcat90-mysql57-jdk8/7

Job Results:

For upstream results, click here.

liferay-continuous-integration commented Jul 20, 2020

john-co commented Jul 20, 2020 • edited

diegonvs commented Jul 21, 2020

john-co commented Jul 21, 2020

liferay-continuous-integration commented Jul 22, 2020

✔️ ci:test:stable - 20 out of 21 jobs passed

✔️ ci:test:relevant - 50 out of 51 jobs passed in 2 hours 11 minutes

Base Branch:

Copied in Private Modules Branch:

1 Failed Jobs:

2 Failed Jobs:

For more details click here.

This pull contains no unique failures.

test-portal-acceptance-pullrequest-batch(master)/modules-unit-project-templates-jdk8

Job Results:

liferay-continuous-integration commented Jul 22, 2020

brunobasto commented Jul 22, 2020

diegonvs commented Jul 22, 2020 • edited

brunobasto commented Jul 22, 2020 • edited

diegonvs commented Jul 22, 2020

liferay-continuous-integration commented Jul 22, 2020

liferay-continuous-integration commented Jul 22, 2020

liferay-continuous-integration commented Jul 22, 2020

liferay-continuous-integration commented Jul 22, 2020

wincent commented Jul 6, 2020 •

edited

john-co commented Jul 20, 2020 •

edited

diegonvs commented Jul 22, 2020 •

edited

brunobasto commented Jul 22, 2020 •

edited