LPS-83714 #837
Closed
LPS-83714 #837
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
|
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. |
|
ci:test:relevant |
❌ ci:test:relevant - 41 out of 48 jobs passed in 1 hour 20 minutes 49 seconds 159 msClick here for more details.Base Branch:Branch Name: master Copied in Private Modules Branch:Branch Name: master-private 7 Failed Jobs:
41 Successful Jobs:
For more details click here.Failures unique to this pull:Failures in common with acceptance upstream results at cd184cd:
Test bundle downloads:
|
|
Just started reviewing :) |
Sent from 
| @@ -120,26 +120,31 @@ | |||
|
|
|||
| <aui:input name="<%= HtmlUtil.getAUICompatibleId(ddmFormValuesInputName) %>" type="hidden" /> | |||
|
|
|||
| <aui:input name="ddmFormFieldNamespace" type="hidden" value="<%= fieldsNamespace %>" /> | |||
There was a problem hiding this comment.
hi @antonio-ortega , since this is only used by DL, is it possible to move this out of the DDM Taglib and keep the logic at DL?
| instance.updateDDMFormInputValue(); | ||
| } | ||
| }, | ||
|
|
||
| _onSubmitForm: function(event) { | ||
| var instance = this; | ||
|
|
||
| instance.updateDDMFormInputValue(); | ||
| if (instance.get('synchronousFormSubmission')) { |
There was a problem hiding this comment.
@antonio-ortega , wouldn't it be better to move this condition checking to the event handlers assignment to avoid calling this method unnecessarily?
| @@ -73,6 +73,12 @@ | |||
| <rtexprvalue>true</rtexprvalue> | |||
| <type>boolean</type> | |||
| </attribute> | |||
| <attribute> | |||
| <name>synchronousFormSubmission</name> | |||
There was a problem hiding this comment.
@antonio-ortega , we cannot directly change *.tld files. The correct process is:
- Add the new input to
liferay-ddm.xml - Run
gradlew buildTaglibs
These steps will update the *.tld files propertly.
|
@antonio-ortega is out of office since, he will come back next week. |
antonio-ortega
force-pushed
the
LPS-83714
branch
from
926e816
to
c832f82
Compare
antonio-ortega
force-pushed
the
LPS-83714
branch
from
c832f82
to
11ebb0c
Compare
|
Just started reviewing :) |
|
Hey @antonio-ortega , I've added the fix related to the taglibs + your commits on this PR: #889. Can you take a look there? Thanks! I'm closing this. |
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.
Hi @rafaprax,
Can you review this PR?
You can find further information in LPS-83714.
The solution is based on this proposal by @natocesarrego.
First commit is enough to solve the issue. Second commit is to avoid execute useless code. Don't hesitate to ask me any question.
Thanks in advance.
Regards,