LPS-139246 - Create specific autocomplete for ddm #1489
Conversation
|
Please only forward necessary changes to Brian Chan during stabilization. Nonurgent changes should wait until the ongoing DXP 7.4 GA1 and Portal 7.4 GA4 release has been completed. For more details on the release timeline and status, see product-delivery. |
|
CI is automatically triggering the following test suites:
|
liferay-continuous-integration
added
ci:test:sf - success
and removed
ci:test:sf - pending
labels
✔️ ci:test:sf - 1 out of 1 jobs passed in 3 minutesClick here for more details.Base Branch:Branch Name: master Sender Branch:Branch Name: LPS-139246-3 1 Successful Jobs:For more details click here. |
|
Jenkins Build:test-portal-source-format#450 Jenkins Report:jenkins-report.html Jenkins Suite:sf Pull Request:liferay-frontend#1489 Testray Routine:EE Pull Request Testray Build:[master] ci:test:sf - bryceosterhaus > liferay-frontend - PR#1489 - 2021-09-22[13:59:19] Testray Importer:publish-testray-report#1484 |
✔️ ci:test:stable - 11 out of 11 jobs passed❌ ci:test:relevant - 25 out of 28 jobs passed in 2 hoursClick here for more details.This pull is eligible for reevaluation. When this upstream build has completed, using the following CI command will compare this pull request result against a more recent upstream result: ci:reevaluate:1327401_483 Base Branch:Branch Name: master Upstream Comparison:Branch GIT ID: ddf229019900a7f664471fcfd14cd8aebacf3c2a ci:test:stable - 11 out of 11 jobs PASSED11 Successful Jobs:
ci:test:relevant - 25 out of 28 jobs PASSED3 Failed Jobs:25 Successful Jobs:
For more details click here.Failures unique to this pull:
Failures in common with acceptance upstream results at ddf2290:
Test bundle downloads:
|
|
Jenkins Build:test-portal-acceptance-pullrequest(master)#483 Jenkins Report:jenkins-report.html Jenkins Suite:relevant Pull Request:liferay-frontend#1489 Testray Routine:EE Pull Request Testray Build:[master] ci:test:relevant - bryceosterhaus > liferay-frontend - PR#1489 - 2021-09-22[14:03:32] Testray Importer:publish-testray-report#1493 |
|
ci:test:relevant |
|
Jenkins Build:test-portal-acceptance-pullrequest(master)#591 Jenkins Report:jenkins-report.html Jenkins Suite:relevant Pull Request:liferay-frontend#1489 Testray Routine:EE Pull Request Testray Build:[master] ci:test:relevant - bryceosterhaus > liferay-frontend - PR#1489 - 2021-09-22[22:51:57] Testray Importer:publish-testray-report#1766 |
| setSelectedValue(''); | ||
| setInputValue(event.target.value); |
There was a problem hiding this comment.
@bryceosterhaus because it is a Forms field, you need to call onChange here so that the Forms can update the context, so it will be able to submit correctly for example, or if it is in the builder it will be able to preview in the builder what is typed into the predefined value field for example on the Sidebar.
| initialLabel, | ||
| initialValue, | ||
| initialLabel = '', | ||
| initialValue = '', | ||
| inputName, | ||
| itemsKey, | ||
| itemsLabel, | ||
| labelKey = 'label', | ||
| name, | ||
| onChange, | ||
| readOnly, | ||
| required, | ||
| value, | ||
| valueKey = 'value', |
There was a problem hiding this comment.
These behaviors for the Field seem a little strange, normally the fields have a common contract.
valuewill receive the current valuepredefinedValueis the initial value that is used when you don't havevalueonChangethe callback will update the value in the Forms
There are other properties but they are not related to this case here.
| <input | ||
| id={inputName} | ||
| name={inputName} | ||
| type="hidden" | ||
| value={selectedValue} | ||
| /> |
There was a problem hiding this comment.
I think I don't even need this hidden input here because FieldBase will take care of that because it also handles various values depending on the language.
| itemsKey={itemsKey} | ||
| itemsLabel={itemsLabel} | ||
| }) { | ||
| const [selectedValue, setSelectedValue] = React.useState(initialValue); |
There was a problem hiding this comment.
There is usually an idea that a Field's value state is bidirectional, it maintains an internal state and is also updated with some value received via value because this component is also used as a preview in the builder and is also used in Sidebar.
The Sidebar and the user view happens what is called evaluation, every time the user types a value in the field and calls onChange it makes a call to the backend to validate the properties of the field, there are other things that it validates too, it can happen the value will change and you will have to update with what came from the backend. But for this case, I think it doesn't exist.
Usually, the useSyncValue hook is used which will take care of this.
| itemsKey={itemsKey} | ||
| itemsLabel={itemsLabel} | ||
| }) { | ||
| const mutatedRef = React.useRef(false); |
There was a problem hiding this comment.
Just a question about it, is this to help you switch when to use value or initial/predefined value? What problems did you have?
There was a problem hiding this comment.
Yeah this was to make sure the initial/predefined is only there when the input hasn't been modified. Since the onChange and value come from outside this component, we aren't able to pass it as an initial state.
Otherwise if we do something like value={value || predefinedValue} it would get weird if the user clears the input and then it would render the predefined again.
There was a problem hiding this comment.
Hmm yeah makes sense, a good strategy.
bryceosterhaus
force-pushed
the
LPS-139246-3
branch
from
192c9be
to
e74fa43
Compare
|
ci:test:sf |
|
ci:test:relevant |
✔️ ci:test:sf - 1 out of 1 jobs passed in 3 minutesClick here for more details.Base Branch:Branch Name: master Sender Branch:Branch Name: LPS-139246-3 1 Successful Jobs:For more details click here. |
|
Jenkins Build:test-portal-source-format#262 Jenkins Report:jenkins-report.html Jenkins Suite:sf Pull Request:liferay-frontend#1489 Testray Routine:EE Pull Request Testray Build:[master] ci:test:sf - bryceosterhaus > liferay-frontend - PR#1489 - 2021-09-24[09:06:54] Testray Importer:publish-testray-report#886 |
|
Jenkins Build:test-portal-acceptance-pullrequest(master)#326 Jenkins Report:jenkins-report.html Jenkins Suite:relevant Pull Request:liferay-frontend#1489 Testray Routine:EE Pull Request Testray Build:[master] ci:test:relevant - bryceosterhaus > liferay-frontend - PR#1489 - 2021-09-24[09:06:59] Testray Importer:publish-testray-report#895 |
|
ci:forward |
|
CI is automatically triggering the following test suites:
The pull request will automatically be forwarded to the user
|
|
Skipping previously passed test suites: |
|
All required test suite(s) passed. |
|
Pull request has been successfully forwarded to brianchandotcom#107771 |
issues.liferay.com/browse/LPS-139246
Previous PR: #1471
Alright, I decided to re-work this and go for the simple solution for now. The goal here was to remove the dependency of
commerce-frontend-js/components/autocomplete/AutocompletefromObjectRelationship.js.Rather that refactor the commerce code, I instead just created a simple autocomplete solution for DDM.
Let me know if you see any issues with this.
I also removed the "value" attribute from the backend because it was redundant of initialValue.
cc @marco-leo @FabioDiegoMastrorilli