Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optionally allow the use of external DOMs #209

Closed
wants to merge 2 commits into from

Conversation

visigoth
Copy link

External DOMs like jsdom are more featureful than the DOM present in ink.
Features like focus and event dispatch can allow for much richer UIs with react.

These commits allow the client to create a DOM and specify its objects to the
render method via options.

Additionally, when the external DOM is available, it is assumed it supports
event dispatch and so ink uses a keyboard event dispatcher.

This enables the use of features like react-hotkeys and other event-based
functionality. For instance, this opens the door to mouse events being
delivered.

@visigoth
Copy link
Author

darn, i was hoping i could make ReactDOM work well enough, but unfortunately, the use of unstable__transformChildren makes this difficult since ReactDOM rejects unknown props on DOM elements. given there are other packages (like ink-box) that use it, it makes it difficult to rename it. forking react-dom isn't really a valid path forward either.

my ultimate goal was to make it easier to integrate key input events as well as events like focus. but, it seems like the only real way to go is to write the appropriate renderer glue in the reconciler :(

@sindresorhus
Copy link
Collaborator

given there are other packages (like ink-box) that use it, it makes it difficult to rename it.

What would you rename it to? I control ink-box, so that's not an issue.

@vadimdemedes
Copy link
Owner

Thank you for the huge effort you've put in this PR, but I'm not sure I would want to merge it in current form. If we were to decide to move to jsdom, we'd go all-in and remove our own implementation of naive DOM. I would advise to always open an issue to discuss such huge changes before writing code, to ensure you don't end up wasting your time :)

As for focus management, I'm a little confused here, how would jsdom help with focus management in the terminal? Unless I'm misunderstanding the idea!

Regarding mouse events, I don't think I would want to have support for it in Ink, at least in the foreseeable future. Mouse support works great in ncurses-based CLIs (see react-blessed), because of their fullscreen approach to rendering UIs. Ink's output, however, just keeps "adding rows" to the terminal and it can go way beyond the terminal height (viewport), which makes it impossible to determine where exactly user is clicking.

@visigoth
Copy link
Author

What would you rename it to? I control ink-box, so that's not an issue.

react-dom allows custom DOM properties when they are all lowercase. so, any all-lowercase custom prop would be fine. i would simply propose lowercasing the whole thing to unstable__transformchildren or unstable__transform_children

As for focus management, I'm a little confused here, how would jsdom help with focus management in the terminal? Unless I'm misunderstanding the idea!

it's not that focus management is somehow specific to jsdom, but that an existing DOM implementation already has the abstractions to manage event listeners, event bubbling, etc. and two of those events are blur and focus, which allows the document to maintain focus. finally, there is an activeElement property, so something in the DOM actually is marked as "focused" (i.e. the entry point for event dispatch and computing the bubbling path).

that can all be implemented in ink's DOM, of course. just seemed nice that something already had it.

Ink's output, however, just keeps "adding rows" to the terminal and it can go way beyond the terminal height (viewport),

hm, that's interesting. i didn't realize the philosophy here. i'd really like to be thinking about my react app as living inside a viewport as i want things to change. what about something like ink-select? it has a bunch of options and selects among them, requiring re-rendering of what's currently on the terminal.

i gave blessed and react-blessed a try, but i ran into a bunch of issues getting what i wanted working. i will re-evaluate it to understand more detail of what's not possible over there that brought me to ink.

@visigoth
Copy link
Author

visigoth commented Jul 5, 2019

i went back to playing around with react-blessed. it is more of what i'm looking for. as you say, it is more viewport-based. however, there are some deficiencies:
1/ event dispatch doesn't follow react (although it doesn't in ink either). i created a PR with the work to implement react SyntheticEvents in react-blessed: Yomguithereal/react-blessed#98
2/ it does not implement any layout like ink does with yoga-layout. i guess i'll take a look at integrating that next, hopefully by just translating layout to positional traits i can pass to blessed.

@vadimdemedes
Copy link
Owner

Glad you found a tool that suits your needs! I'm going to close this PR for now, but feel free to come back with ideas, suggestions and PRs in the future ;)

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

Successfully merging this pull request may close these issues.

3 participants