Skip to content
This repository has been archived by the owner on May 5, 2021. It is now read-only.

Move locate to DOM Renderers #144

Open
Zynton opened this issue Mar 26, 2020 · 0 comments
Open

Move locate to DOM Renderers #144

Zynton opened this issue Mar 26, 2020 · 0 comments

Comments

@Zynton
Copy link
Contributor

Zynton commented Mar 26, 2020

We need to move this locate stuff. It is DOM stuff so it should be on the renderer. If it was ideed on the renderer, you could simply read the engine.renderings object rather than having to "observe" your DOM surroundings. Something like this:

class FontAwesomeDomRenderer {
    locate(domNode: Node, offset: number, node: VNode): [this, RelativePosition] {
        const rendering = this.engine.renderings.get(node);
        if (domNode === rendering[rendering.length - 1]) {
            // We are targetting the invisible character AFTER the
                // fontawesome node.
            if (offset === 0) {
                // Moving from after the fontawesome node to before it.
                // (DOM is `<invisible/><fontawesome/>[]<invisible/>` but
                // should be `[]<invisible/><fontawesome/><invisible/>`).
                return [this, RelativePosition.BEFORE];
            } else {
                // Stay after the fontawesome node.
                return [this, RelativePosition.AFTER];
            }
        } else if (domNode === rendering[0]) {
            // We are targetting the invisible character BEFORE the
            // fontawesome node.
            if (offset === 1) {
                 // Moving from before the fontawesome node to after it.
                // (DOM is `<invisible/>[]<fontawesome/><invisible/>` but
                // should be `<invisible/><fontawesome/><invisible/>[]`).
                return [this, RelativePosition.AFTER];
            } else {
                // Stay before the fontawesome node.
                return [this, RelativePosition.BEFORE];
            }
        }  else {
            // We should never reach this. It's a fallback to prevent a crash in
            // the eventuality that it would.
            return [this, RelativePosition.BEFORE];
        }
    }
}

In particular, your code will fail in the event that some node that is not a FontAwesomeNode ends up rendering itself using a node that your regex would recognize as being a fontawesome. If it does, the first invisible character would be mistaken for the second one because its "previousSibling" would be recognized as a fontawesome even though it is not the one that this invisible character is "attached" to. Hence my suggestion of using the renderings object to specifically work on the nodes rendered by that VNode and only that.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant