Edit text block inline

[MAX-786]

Fixes #28 , #29 #5

Demo:

https://github.com/collective/volto-hydra/assets/99530996/d921f5d7-f314-48d3-b353-feb269b44caa

[MAX-786]

currently you can add/remove text inline YAY! :) Next is which is serious part, is able to change its format and enabling pressing enter on keyboard will do stuff conditionally.

[djay]

Nice. I really should work out how to setup deployments for feature branches.

I think you can work on selection and formatting in another pr?

[MAX-786]

Yepp Surely, I'll just document some findings here then we'll be ready to move it.

[MAX-786]

Things getting complex now so I am just gonna write a comment here so I remember what approach I used for the problem i faced so if we decided to change things or are unhappy with results we can comeback here for a revision.

So the first tough part was to keep track of nodeIds nd marking the whole formData was too expensive task nd so I decided it to mark only that object with selected block id at a time.

Moreover, it needs to be pointed out that numbering of HTML nodes at frontend side should be in preorder fashion i.e. mark the parent and then mark its children one by one till u reach the leaf node then its immediate parent's neighbour then it's children till u reach its leaf node and so on. Something like:

{
    "@type": "slate",
    "plaintext": " Plone is a content managemen...",
    "value": [
        {
            "children": [
                {
                    "text": "",
                    "nodeId": 2
                },
                {
                    "children": [
                        {
                            "text": "Plone",
                            "nodeId": 4
                        }
                    ],
                    "data": {
                        "url": "https://plone.org/"
                    },
                    "type": "link",
                    "nodeId": 3
                },
                {
                    "text": " is a content managemen... ",
                    "nodeId": 5
                }
            ],
            "type": "p",
            "nodeId": 1
        }
    ],
    "nodeId": 0
}

And also, the very confusing part was keeping the formData from adminUI and frontend (hydra.js) at sync and the update should be asynchronous otherwise collisions were creating unexpected results. So, I decided to stop sending the data from adminUI to frontend whenever inline-editing is started and set a focusout listener on selected block's div to resume sending the data from adminui to frontend.

[djay]

Not sure I fully understand but if i think what it means is that yes, once someone starts editing text inline then it's a special mode where the nodeids get rendered. And if the user is making changes then those get sent back but there shouldn't be a need for full updates to the whole page during that time. As soon as the inline loses focuses then it should leave this mode and go back to normal whole page updating.

Means you only need to keep track of the nodeids for a limited time. If they lose focus and do something in the sidebar then you don't have to keep track anymore.

[MAX-786]

As soon as the inline loses focuses then it should leave this mode and go back to normal whole page updating.

Yepp, this is what happening, opps I just saw i forgot to attach a demo video. I will do it right away so it will be more clear.

[djay]

@MAX-786 as discussed on the meeting requiring the integrator to use a algorithm to number the nodes is not very friendly. We always are aiming for the simplest to explain instructions to the integrator. So I think you need some mechanism where the integrator gets the node IDs given to them. Might be easier to think of them as uuid. We also discussed how when the admin UI creates a new node, like making something bold, the id for that has to come from somewhere. It probably makes sense the id is generated on the adminui side? I'm sure you will work out the right way to do it when you get bold working.

[djay]

@MAX-786 remember to write out the developer instructions in the readme as you go rather than at the end. It makes it easier to review since the readme is how we measure the DX. And also it helps you think about it it's as easy as it could be.

[MAX-786]

We always are aiming for the simplest to explain instructions to the integrator. So I think you need some mechanism where the integrator gets the node IDs given to them. Might be easier to think of them as uuid.

So, to find a way to lesser the work for integrator, once the nodeids are added to the json to the selectedBlock, I am calling the callback which was given to onEditChange for live updates . LIke:

  /**
   * Method to add border, ADD button and Quanta toolbar to the selected block
   * @param {Element} blockElement - Block element with the data-block-uid attribute
   */
  selectBlock(blockElement) {
    // Remove border and button from the previously selected block
    if (this.currentlySelectedBlock) {
      this.deselectBlock(this.currentlySelectedBlock);
    }
    const blockUid = blockElement.getAttribute('data-block-uid');
    this.selectedBlockUid = blockUid;

    this.makeBlockContentEditable(blockElement);
    this.observeBlockTextChanges(blockElement);
    if (this.formData) {
      this.formData.blocks[blockUid] = this.addNodeIds(
        this.formData.blocks[blockUid],
      );
      this.setDataCallback(this.formData); // updating frontend data with nodeIds data
    }

What it does is that now frontend components will have access to nodeIds through data too. So, instead of adding a counter for nodeIds themselves, they can now use opitonal chaining to access nodeIds from data itself, like if they are using Slate to render txt block:

const SlateBlock = ({ value }) => {
  const editor = React.useMemo(() => withReact(withHistory(createEditor())), []);
  const renderElement = ({ attributes, children, element }) => {
    if (element.type === "link") {
      return (
        <a href={element.data.url} {...attributes} data-hydra-node={`${element?.nodeId}`}> // No need for the counter, now it is given by hydra.js itself
          {children}
        </a>
      );
    }

    const Tag = element.type;
    return <Tag {...attributes} data-hydra-node={`${element?.nodeId}`}>{children}</Tag>;
  };

  const renderLeaf = ({ attributes, children }) => {
    return <span {...attributes} data-hydra-node={`${children.props.leaf?.nodeId}`}>{children}</span>; // similarly for the leaf node
  };

  const initialValue = value || [{ type: "p", children: [{ text: "" }] }];
  editor.children = initialValue;
  return (
    <Slate editor={editor} initialValue={initialValue}>
      <Editable renderElement={renderElement} renderLeaf={renderLeaf} readOnly />
    </Slate>
  );
};

[MAX-786]

@djay I have updated readme, Can you please check readme and see if DX looks understandable and doable or not?

[MAX-786]

@JeffersonBledsoe I updated the lockfile and yet it gives the error :(

I found something relevant to this ig https://github.com/pnpm/pnpm/issues/6312 Is there any easy way to fix it?

[MAX-786]

@djay I see that you have updated the readme instruction including from here too, I'll add some tweaks and also update our hydrajs so the it does what instructions say.

[MAX-786]

CUrrently this PR lets you do the following:

• [x] Edit a text field on frontend if you have added the attribute data-editable-field=... to it.
• [x] Edit rich text field if you have added data-editable-field='value' and added nodeids as stated in the readme instructions

I'll open up a new PR for the formatting of text .

[MAX-786]

@JeffersonBledsoe from what i can find is that, this lockfile test fail is due to 'volto-slate' package's lockfile not updated with its package.json . But i think merging this PR will get deployed anyways, right? even if it does or does not, we should inform volto team about this package not updated error.

[JeffersonBledsoe]

@MAX-786 The lockfile was actually out-of-date. It looks like the issue arose from the recent Volto upgrade being merged into this branch. If you don't re-fetch the core folder with mrs-developer locally, pnpm locally will think everything is okay, whilst the CI will fail as it is fetching a fresh version each time. I've updated the lockfile and tests are passing again in #115