Repeat with different variants #205
Conversation
|
Hello, I'm the AEM Code Sync Bot and I will run some actions to deploy your branch and validate page speed.
|
|
|
component-models.json
Outdated
| "value": "addRemoveAll" | ||
| }, | ||
| { | ||
| "name": "Add & Remove Last", |
There was a problem hiding this comment.
Add and Remove actions on last instance
There was a problem hiding this comment.
as discussed we are going with same Add & Remove Last
There was a problem hiding this comment.
The display names may be quite confusing for the author here..
Add & Remove on all instances
Add & Remove on the last instance only
And does all instances mean that one can insert an instance in between 2 instances as well?
There was a problem hiding this comment.
Post discussions, There will be only two variants, No actions and Add & Remove Buttons
| @@ -1,12 +1,15 @@ | |||
| <form data-action="undefined" novalidate="" data-source="sheet" data-rules="false"> | |||
| <div data-min="undefined" data-max="undefined" class="repeat-wrapper"> | |||
| <div data-min="0" data-max="undefined" data-variant="addRemoveAll" class="repeat-wrapper"> | |||
There was a problem hiding this comment.
I think we should fix all undefined now.
jalagari
force-pushed
the
repeat-variant
branch
from
e779cbc to
d653d3a
Compare
jalagari
force-pushed
the
repeat-variant
branch
from
d653d3a to
5ec36f8
Compare
jalagari
force-pushed
the
repeat-variant
branch
from
5ec36f8 to
c314099
Compare
jalagari
force-pushed
the
repeat-variant
branch
from
c314099 to
68a7715
Compare
jalagari
force-pushed
the
repeat-variant
branch
from
68a7715 to
cd61abb
Compare
jalagari
force-pushed
the
repeat-variant
branch
from
cd61abb to
9041601
Compare
jalagari
force-pushed
the
repeat-variant
branch
from
9041601 to
ba11a36
Compare
component-models.json
Outdated
| "value": "addRemoveAll" | ||
| }, | ||
| { | ||
| "name": "Add & Remove Last", |
There was a problem hiding this comment.
The display names may be quite confusing for the author here..
Add & Remove on all instances
Add & Remove on the last instance only
And does all instances mean that one can insert an instance in between 2 instances as well?
| export function insertAddButton(wrapper, form) { | ||
| const actions = document.createElement('div'); | ||
| actions.className = 'repeat-actions'; | ||
| const addLabel = wrapper?.dataset?.repeatAddButtonLabel || 'Add'; |
There was a problem hiding this comment.
We will have to handle both of these props in the translation layer as well since these are labels. Pls log a ticket for the same.
Also, the recommended way is to prefix these props with fd: as these are OOTB props to avoid their clash with custom props.
There was a problem hiding this comment.
And we should have this registered in the sling models also.
There was a problem hiding this comment.
There are less chances of conflicting as they need to add properties in _panel.json
There was a problem hiding this comment.
We'd need it anyways for translation. Let's make sure this does not gets lost.
There was a problem hiding this comment.
Conflicting can appear for migration use cases in future. We are talking about jcr persistence here.
There was a problem hiding this comment.
@nit23uec - Content will get migrated but they need to updated panel definition in edge delivery repo and also need to rebuild component here so they can take care of it.
Repeating again the main reason I wanted to avoid it part of CC because of following reasons
Customer who are in old build will not get this feature until they upgrade there CC and run pipeline
Please always provide the GitHub issue(s) your PR is for, as well as test URLs where your change can be observed (before and after):
Fix #
Test URLs: