Show border, addButton and Quanta Toolbar over Block when selected

MAX-786 commented 3 weeks ago

Fixes #24, fixes #16, fixes #25, fixes #27, fixes #26

Features:

All these 3 things (border, add & delete btn) are injected by Bridge(hydra.js) only when it is loaded inside Hydra and it is in edit mode.
Click on block and border and buttons will appear.
Click on add button and it will add a slate (default) block below the selected block and the border will shift to that new block (now using mutation observer).
You can also edit text of slate blocks from sidebar and it is sent to iframe in real time.
Color and design are subjected to change as per review.

Goals in order of execution:

[x] Border is rendered by hydra.js and it is same for all block types. (as discussed this is enough for the scope of this PR)
[x] Add button and delete button injected by hydra.js.
[x] Add button adds a text block block and border is moved to new block.
[x] Now Add button opens up a BlockChooser Menu and it is rendered by adminUI over the top of iframe.
[x] Clicking on block will show up quanta toolbar injected by hydra.js and will have drag btn (not functional) and three dot menu which will include Remove btn. (As for now it's the same for mobile)
[x] Placing the components as per quanta design .

Changes to previous implementations:

Instead of using redux state, Now using onSelectBlock() method to update sidebar when block is selected in Iframe.
Using props from Form.jsx to addBlock & deleteBlock from the formData.

Current problems:

(please let us know if you find any)

Current Design:

https://github.com/collective/volto-hydra/assets/99530996/b76aa98c-b65c-4c18-a008-58360f4865df

djay commented 3 weeks ago

as mentioned in the meeting. The add popup doesn't have to match the quanta styling (other than where its placed on screen) The quanta toolbar would just have the drag icon (not working/disabled) and the ... menu. There are mobile aspects to how the quanta toolbar and add menu are placed. but maybe could split those out

JeffersonBledsoe commented 2 weeks ago

@MAX-786 I haven't figured out why yet, but the issue with the onDeleteBlock call not working properly is that the this.props.globalData isn't being updated. So when the component updates, it replaces the existing form data with the stale global data, causing it to bring the old block back. See https://github.com/collective/volto-hydra/blob/b8aa970c0aec766de59af8fc937d537b5a518e83/packages/volto-hydra/src/customizations/components/manage/Form/Form.jsx#L286-L293 for where this happens. this.props.globalData is updated by calling this.props.setFormData. So either we've removed a call to this, or something we have changed is causing this call to get out of sync with the actual form data

MAX-786 commented 2 weeks ago

@JeffersonBledsoe Thanks and Yepp, I fixed it!!! Well globalData was getting updated but not inside the onDeleteBlock() method defined in Iframe component. Read Below for more details! The problem was in the Iframe component, here See: https://github.com/collective/volto-hydra/blob/4efcbf572978913a01aaab7392bdba8f71d51c31/packages/volto-hydra/src/components/Iframe/View.jsx#L75-L88

You can see onDeleteBlock was defined outside of the useEffect, which means it captured and used the properties state from the time it was defined. If properties updated, onDeleteBlock was still using the old properties state, leading to stale or incorrect state usage.

But when i moved the onDeleteBlock function inside the useEffect, it gets redefined every time the useEffect runs. This ensures that it always captures the latest state and props, including the updated properties.

I hope this is the correct way to do this. :)

MAX-786 commented 2 weeks ago

I will be pushing the code now, with addblock feature using the BlockChooser form and deleteBlock btn still at the right side of the block which both works as intented!! (since quanta toolbar is now the next target of this PR and i'll be moving delete btn in quanta toolbar)

JeffersonBledsoe commented 2 weeks ago

@JeffersonBledsoe Thanks and Yepp, I fixed it!!! Well globalData was getting updated but not inside the onDeleteBlock() method defined in Iframe component. Read Below for more details!

The problem was in the Iframe component, here See:

https://github.com/collective/volto-hydra/blob/4efcbf572978913a01aaab7392bdba8f71d51c31/packages/volto-hydra/src/components/Iframe/View.jsx#L75-L88

You can see onDeleteBlock was defined outside of the useEffect, which means it captured and used the properties state from the time it was defined. If properties updated, onDeleteBlock was still using the old properties state, leading to stale or incorrect state usage.

But when i moved the onDeleteBlock function inside the useEffect, it gets redefined every time the useEffect runs. This ensures that it always captures the latest state and props, including the updated properties.

I hope this is the correct way to do this. :)

Good catch on the stale closure! I think the more "React" fix would be to set properties as one of the dependencies within the use effect, but I'd need to look more closely when I get back. Feel free to continue with this approach for now though if it works so we can continue the project :)

MAX-786 commented 2 weeks ago

@JeffersonBledsoe is it okay technically to merge it? Like after Dylan's review?

JeffersonBledsoe commented 2 weeks ago

I haven't looked properly, but go for it if the functionality is there as we can improve it later

MAX-786 commented 2 weeks ago

Yeppp same thought :) Then I'll wait for Dylan's review.

djay commented 2 weeks ago

@MAX-786 he is also using grey on the drag icon to help the toolbar stand out. Would be good to add that.

djay commented 2 weeks ago

@MAX-786 looks good. We can merge it I think

MAX-786 commented 2 weeks ago

Merging and closing it!

collective / volto-hydra