This is part of #2877927: Show add widget between paragraphs and it's also one part of bigger scope #2236905: [META] Nicer UI / Icons for "Paragraph type" + "Add another Paragraph".
Problem/Motivation
After defining next steps in order to support "Add in between" functionality, we came to the conclusion that paragraphs will support adding a new paragraph to the desired position. In the first step, it will not provide UI for that and UI can be provided by sub-modules or other integrations. UI should be purely done in JavaScript and underlying paragraph functionalities should provide easy integration.
Proposed resolution
Add a new hidden field for Modal dialog mode that will pass information about the position where new paragraph should be added.
Remaining tasks
- add new hidden field for "Modal" mode
- add extensive testing for that
Comment | File | Size | Author |
---|---|---|---|
#11 | 2944372_interdiff_11_9.txt | 1005 bytes | mtodor |
#11 | 2944372_11.patch | 19.75 KB | mtodor |
| |||
#9 | 2944372_interdiff_9_7.txt | 5.76 KB | mtodor |
#9 | 2944372_9.patch | 19.75 KB | mtodor |
| |||
#7 | 2944372_interdiff_5_7.txt | 9.22 KB | mtodor |
Comments
Comment #2
miro_dietikerLooking very much forward to it. :-)
Comment #3
mtodor CreditAttribution: mtodor at Thunder commentedHere is a patch with the proposed solution.
Following changes are made:
Comment #5
mtodor CreditAttribution: mtodor at Thunder commentedFixed incorrect fetching of submitted delta value.
Comment #6
BerdirWould it be easier to just set add_more_delta to $max_delta (or maybe support an explicit NULL argument) to have it added at the end?
I'm also not quite sure yet about the name. createNewPosition() sounds very generic. I'm not quite sure how to call it, maybe something more specific to what we are doing, something involving the deltas...
Comment #7
mtodor CreditAttribution: mtodor at Thunder commented@Berdir I'm not sure what you meant with
But I agree, that part with if/else wasn't nice, so I moved that logic into the function, since "else" part is already done in the function too and it's also nice to have all delta constrains in one place.
Here are changes for this interdiff:
Comment #8
BerdirFor readability, I personally prefer to keep function calls on a single line and instead move some of those long arguments to a variable on a separate line like $field_path and $add_more_delta (as before)
this is to test that a negative delta is ignored and used as 0?
what about prepareDeltaPosition()?
Or: updateWidgetStateDeltas()
And the description could be something like
Prepares the widget state to add a new paragraph at a specific position.
and $delta could be $new_delta or $add_$delta?
I also think it is int|null, not mixed?
do we really need 3 different text-ish paragraph types for this test? Defining them all with a different text adds quite bit of overhead.
A nested does make sense to test nested scenarios.
ah, so you use that to make it easier to test the expected order. Fair enough, ignore that part then.
Comment #9
mtodor CreditAttribution: mtodor at Thunder commented@Berdir tnx for review. I have adjusted everything from #8.
prepareDeltaPosition
works. I have added comment.prepareDeltaPosition
because we are adjusting there everything required for that task, not just widget state.$new_delta
int|mixed
because it can be also string (there is also test for that, search "some_text"). Point there is, withint|mixed
we are telling that - int value should be used, but everything is accepted and it will workComment #10
miro_dietikerCrosspost...
Awesome, this reads so straight and shares so much code with duplicate. :-)
Agree, simple is beautiful, but the common approach would be to add unique text to each Paragraph. You could then easily check for expected text in items or on output. Is the current approach somehow easier?
Checked code in detail and the delta handling feels perfectly right. Didn't test though.
I think i would move this down to where the variable is used, at least after the loop.
@mtodor Will you look into consuming this new API directly with Thunderspecific JS or also try to help us bring the button to the actions first?
Comment #11
mtodor CreditAttribution: mtodor at Thunder commented@miro_dietiker - tnx for review. I have moved the line to the bottom part, nice catch.
Regarding testing: Yes - it's easier and simpler. Tests are executed faster and they also test more explicitly new functionality.
Regarding using new API: We will probably first migrate functionality we already have, more or less what is provided in #2877927: Show add widget between paragraphs.
Comment #13
miro_dietikerAwesome this is ready to unlock the next level. :-)
Thank you for the great work. Committed.
Comment #14
mtodor CreditAttribution: mtodor at Thunder commentedHere is the first use of this API. https://www.drupal.org/project/paragraphs_features