Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LPS-124430 Saves tooltip trigger title on show events #629

Closed
wants to merge 1 commit into from

Conversation

jbalsas
Copy link

@jbalsas jbalsas commented Dec 17, 2020

From the commit itself:

Before, the change of title to data-restore-title happened based on the component rendering and state, making it fail to execute under certain conditions.

With this change, anytime the mouse moves over a tooltip trigger, its title attribute is moved to a data-restore-title attribute avoiding unnecessary rendering of the native tooltip.

The effect can be seen in this screencast and it happened to all tooltips in DXP:

tooltip

The Problem
To take over the native tooltips, we move title attributes to data-restore-title when we enter a trigger element. This attribute is then restored back to title when we abandon it.

Previous logic depended on a useEffect(... [target]) and some internal conditions to execute the saveTitle method while restoreTitle was running unconditionally when the trigger was abandoned (like to move into the tooltip).

The Fix
We simply move the saveTitle execution to the initial show trigger. This means there are more DOM changes but the DOM state remains consistent through interactions.

Test Plan

  • Hover over an icon in DXP and wait until the tooltip shows
  • Move the mouse over the tooltip
  • Move it back over the icon
  • Wait for the native tooltip to appear

Additional Reference
Just because I've kept this around all this time, in case someone goes into the implementation and wonders what's with all the craziness... this is the tooltip state diagram:

tooltip

@liferay-continuous-integration
Copy link
Collaborator

CI is automatically triggering the following test suites:

  •     ci:test:relevant
  •     ci:test:sf

@liferay-continuous-integration
Copy link
Collaborator

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

Click here for more details.

Base Branch:

Branch Name: master
Branch GIT ID: ff0e5e147ea14773fc626b3a4024d041bfefb138

Sender Branch:

Branch Name: LPS-124430
Branch GIT ID: 5c75543df5642a85527c0c2e2fbb826f7a2f0001

1 out of 1jobs PASSED
1 Successful Jobs:
For more details click here.

Before, the change of `title` to `data-restore-title` happened based on the
component rendering and state, making it fail to execute under certain
conditions.

With this change, anytime the mouse moves over a tooltip trigger, its
`title` attribute is moved to a `data-restore-title` attribute avoiding
unnecessary rendering of the native tooltip.
@jbalsas
Copy link
Author

jbalsas commented Dec 17, 2020

ci:stop

@jbalsas
Copy link
Author

jbalsas commented Dec 17, 2020

I somehow managed to mangle a one-line commit 🙈

@jbalsas
Copy link
Author

jbalsas commented Dec 17, 2020

ci:stop

@liferay-continuous-integration
Copy link
Collaborator

						<h1>Unable to generate test report.</h1><p>Jenkins encountered the following error while generating test report:</p><pre><code>Sourced file: inline evaluation of: `` 						 							import com.liferay.jenkins.results.parser.Build; 							import co . . . '' : TargetError : at Line: 48 : in file: inline evaluation of: `` 						 							import com.liferay.jenkins.results.parser.Build; 							import co . . . '' : throw e ; 

Target exception: java.lang.RuntimeException: Unable to find properties for git.portal.properties

Base Branch:

Branch Name: master

Branch GIT ID: ff0e5e147ea14773fc626b3a4024d041bfefb138

Job Summary:

Job Link: test-portal-acceptance-pullrequest(master)

For more details click here.

@jbalsas
Copy link
Author

jbalsas commented Dec 17, 2020

ci:test:sf

@jbalsas
Copy link
Author

jbalsas commented Dec 17, 2020

ci:test:relevant

@liferay-continuous-integration
Copy link
Collaborator

@julien
Copy link

julien commented Dec 17, 2020

@jbalsas the other day between two tasks, I saw this one on the issue board and have the exact same change in a branch

@julien
Copy link

julien commented Dec 17, 2020

@jbalsas how did you generate that diagram?

@jbalsas
Copy link
Author

jbalsas commented Dec 17, 2020

@jbalsas the other day between two tasks, I saw this one on the issue board and have the exact same change in a branch

Next time assign the issue to yourself then so we know someone's looking at it! 👀

@jbalsas how did you generate that diagram?

I can't quite remember 😂

@liferay-continuous-integration
Copy link
Collaborator

@wincent
Copy link

wincent commented Dec 17, 2020

Top Giphy result for "state machines":

state-machines

@liferay-continuous-integration
Copy link
Collaborator

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

❌ ci:test:relevant - 20 out of 23 jobs passed in 2 hours 22 minutes

Click here for more details.

Base Branch:

Branch Name: master
Branch GIT ID: ff0e5e147ea14773fc626b3a4024d041bfefb138

Copied in Private Modules Branch:

Branch Name: master-private
Branch GIT ID: 0ff04d9f73a4643abdd2b335581feef26b985b8a

ci:test:stable - 9 out of 9 jobs PASSED
9 Successful Jobs:
ci:test:relevant - 20 out of 23 jobs PASSED
20 Successful Jobs:
For more details click here.

Failures unique to this pull:


Failures in common with acceptance upstream results at 39cc3ed:
  1. test-portal-acceptance-pullrequest-batch(master)/semantic-versioning-jdk8/0
    Job Results:

    0 Tests Passed.
    1 Test Failed.

    1. AXIS_VARIABLE=0,label_exp=!master #447503

      Please fix semantic versioning on jbalsas/LPS-124430

           [exec] 			+   return     void
           [exec] 		+   method     <init>(java.lang.String)
           [exec] 			+   return     void
           [exec] 		+   method     <init>(java.lang.String,java.lang.Throwable)
           [exec] 			+   return     void
           [exec] 		+   method     <init>(java.lang.Throwable)
           [exec] 			+   return     void
           [exec] 		+   method     addSuppressed(java.lang.Throwable)
           [exec] 			+   access     final
           [exec] 			+   return     void
           [exec] 		+   method     clone()
           [exec] 			+   access     protected
           [exec] 			+   return     java.lang.Object
           [exec] 		+   method     equals(java.lang.Object)
           [exec] 			+   return     boolean
           [exec] 		+   method     fillInStackTrace()
           [exec] 			+   return     java.lang.Throwable
           [exec] 		+   method     finalize()
           [exec] 			+   access     protected
           [exec] 			+   return     void
           [exec] 		+   method     getCause()
           [exec] 			+   return     java.lang.Throwable
           [exec] 		+   method     getLocalizedMessage()
           [exec] 			+   return     java.lang.String
           [exec] 		+   method     getMessage()
           [exec] 			+   return     java.lang.String
           [exec] 		+   method     getStackTrace()
           [exec] 			+   return     java.lang.StackTraceElement[]
           [exec] 		+   method     getSuppressed()
           [exec] 			+   access     final
           [exec] 			+   return     java.lang.Throwable[]
           [exec] 		+   method     hashCode()
           [exec] 			+   return     int
           [exec] 		+   method     initCause(java.lang.Throwable)
           [exec] 			+   return     java.lang.Throwable
           [exec] 		+   method     printStackTrace()
           [exec] 			+   return     void
           [exec] 		+   method     printStackTrace(java.io.PrintStream)
           [exec] 			+   return     void
           [exec] 		+   method     printStackTrace(java.io.PrintWriter)
           [exec] 			+   return     void
           [exec] 		+   method     setStackTrace(java.lang.StackTraceElement[])
           [exec] 			+   return     void
           [exec] 		+   method     toString()
           [exec] 			+   return     java.lang.String
           [exec] [Baseline Report] Mode: diff (persisted)
           [exec] Semantic versioning is incorrect while checking /opt/dev/projects/github/liferay-portal/portal-kernel/portal-kernel.jar against /opt/dev/projects/github/liferay-portal/.gradle/caches/modules-2/files-2.1/com.liferay.portal/com.liferay.portal.kernel/9.18.0/471e51c2d8cc477362ba9dd7449b8c3974545660/com.liferay.portal.kernel-9.18.0.jar

@jbalsas
Copy link
Author

jbalsas commented Dec 17, 2020

I don't see how that test can be related... let's give this another 2 and a half hours of our lives just to be sure!

@jbalsas
Copy link
Author

jbalsas commented Dec 17, 2020

ci:test:relevant

@liferay-continuous-integration
Copy link
Collaborator

@wincent
Copy link

wincent commented Dec 17, 2020

I don't see how that test can be related... let's give this another 2 and a half hours of our lives just to be sure!

funny

@liferay-continuous-integration
Copy link
Collaborator

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

❌ ci:test:relevant - 20 out of 23 jobs passed in 1 hour 54 minutes

Click here for more details.

Base Branch:

Branch Name: master
Branch GIT ID: ff0e5e147ea14773fc626b3a4024d041bfefb138

Copied in Private Modules Branch:

Branch Name: master-private
Branch GIT ID: 0ff04d9f73a4643abdd2b335581feef26b985b8a

ci:test:stable - 9 out of 9 jobs PASSED
9 Successful Jobs:
ci:test:relevant - 20 out of 23 jobs PASSED
20 Successful Jobs:
For more details click here.

Failures unique to this pull:


Failures in common with acceptance upstream results at 6f7122e:
  1. test-portal-acceptance-pullrequest-batch(master)/semantic-versioning-jdk8/0
    Job Results:

    0 Tests Passed.
    1 Test Failed.

    1. AXIS_VARIABLE=0,label_exp=!master #459052

      Please fix semantic versioning on jbalsas/LPS-124430

           [exec] 			+   return     void
           [exec] 		+   method     <init>(java.lang.String)
           [exec] 			+   return     void
           [exec] 		+   method     <init>(java.lang.String,java.lang.Throwable)
           [exec] 			+   return     void
           [exec] 		+   method     <init>(java.lang.Throwable)
           [exec] 			+   return     void
           [exec] 		+   method     addSuppressed(java.lang.Throwable)
           [exec] 			+   access     final
           [exec] 			+   return     void
           [exec] 		+   method     clone()
           [exec] 			+   access     protected
           [exec] 			+   return     java.lang.Object
           [exec] 		+   method     equals(java.lang.Object)
           [exec] 			+   return     boolean
           [exec] 		+   method     fillInStackTrace()
           [exec] 			+   return     java.lang.Throwable
           [exec] 		+   method     finalize()
           [exec] 			+   access     protected
           [exec] 			+   return     void
           [exec] 		+   method     getCause()
           [exec] 			+   return     java.lang.Throwable
           [exec] 		+   method     getLocalizedMessage()
           [exec] 			+   return     java.lang.String
           [exec] 		+   method     getMessage()
           [exec] 			+   return     java.lang.String
           [exec] 		+   method     getStackTrace()
           [exec] 			+   return     java.lang.StackTraceElement[]
           [exec] 		+   method     getSuppressed()
           [exec] 			+   access     final
           [exec] 			+   return     java.lang.Throwable[]
           [exec] 		+   method     hashCode()
           [exec] 			+   return     int
           [exec] 		+   method     initCause(java.lang.Throwable)
           [exec] 			+   return     java.lang.Throwable
           [exec] 		+   method     printStackTrace()
           [exec] 			+   return     void
           [exec] 		+   method     printStackTrace(java.io.PrintStream)
           [exec] 			+   return     void
           [exec] 		+   method     printStackTrace(java.io.PrintWriter)
           [exec] 			+   return     void
           [exec] 		+   method     setStackTrace(java.lang.StackTraceElement[])
           [exec] 			+   return     void
           [exec] 		+   method     toString()
           [exec] 			+   return     java.lang.String
           [exec] [Baseline Report] Mode: diff (persisted)
           [exec] Semantic versioning is incorrect while checking /opt/dev/projects/github/liferay-portal/portal-kernel/portal-kernel.jar against /opt/dev/projects/github/liferay-portal/.gradle/caches/modules-2/files-2.1/com.liferay.portal/com.liferay.portal.kernel/9.18.0/471e51c2d8cc477362ba9dd7449b8c3974545660/com.liferay.portal.kernel-9.18.0.jar

@jbalsas
Copy link
Author

jbalsas commented Dec 17, 2020

@john-co, @PablitoBonito could you see if this related after all? Failed twice in a row, so it's either that or already broken upstream 🤔

@liferay-continuous-integration
Copy link
Collaborator

@jbalsas
Copy link
Author

jbalsas commented Dec 17, 2020

Same issue also seen at #636, so forwarding manually!

@jbalsas
Copy link
Author

jbalsas commented Dec 17, 2020

Forwarded at brianchandotcom#97047

@jbalsas jbalsas closed this Dec 17, 2020
@wincent
Copy link

wincent commented Dec 17, 2020

And #632brianchandotcom#97050

@john-co
Copy link

john-co commented Dec 17, 2020

Reviewed. Test failures are unrelated. ✔️

Confirmed issue already exists in upstream: https://liferay.spiraservice.net/16/TestCase/191632.aspx

@jbalsas jbalsas deleted the LPS-124430 branch September 14, 2021 08:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants