jonmbake / react-terminal-ui

A terminal react component with support for light and dark modes.
https://jonmbake.github.io/react-terminal-ui/demo/
MIT License
212 stars 32 forks source link

Could you expose the function to change the input value #5

Closed Jean-PhilippeD closed 3 years ago

Jean-PhilippeD commented 3 years ago

I use a support of react-hot-key so I can intercept such keys like ctrl+c So in my case, I'd like to be able to manipulate the input field when user hit ctrl+c (or ctrl+d or maybe get history of command with up arrow. .) so I could add the interrupt information on last line and reset the input:

prompt> echo "foo" ^C
prompt>

Maybe we could have a props acting like a wrapper :

const [wrapper, setWrapper] = useState((value) => return value);

const clearInput = () => {
  setWrapper((value) => return '')
}
// On ctrl+c hit, call clearInput

<Terminal valueWrapper={wrapper} />

And in Terminal, the updateCurrentLineInput would have to call the wrapper to transform the value if valueWrapperis set.

I'm not sure about the method, you may have a better solution.

jonmbake commented 3 years ago

Hi @Jean-PhilippeD . Thanks for opening up this issue. I see your use case and it is definitely doable. I would like to do it in a way as to not expose the internal input state; a new prop input value wrapper might work. Let me think about this one for a bit. Feel free to submit a PR with your suggestion.

jonmbake commented 3 years ago

@Jean-PhilippeD What do you think about simply adding a startingInput: string prop? like:

<Terminal startingInput='echo "foo"' />

The terminal could be cleared by:

<Terminal startingInput="" />

Since the Terminal would only update when the startingInput changed, we may need to make the type of startingInput to be a Symbol, which is kind of weird.

Jean-PhilippeD commented 3 years ago

Yes I think it can do the job but why do you need to make the type to be a Symbol ?

jonmbake commented 3 years ago

If it's a String type, subsequent clearInputs would not work, e.g.

const [startingInput, setStartingInput] = useState("");
const clearInput = () => {
  setStartingInput("");
}
<Terminal startingInput={startingInput} />

Since the startingInput prop is not changing values.

Jean-PhilippeD commented 3 years ago

I'm not sure I really understand the matter, I still need to learn about react and js..

I thought that if you set

useEffect(() => {
  //change the starting input
}, [props.startingInput])

That way, the input would change only if props change ?

jonmbake commented 3 years ago

I was going to update the Terminal component def with:

useEffect(() => {
    setCurrentLineInput(props.startingInput);
  }, [props.startingInput]);

so setCurrentLineInput would only get called when props.startingInput changes. I'm not sure if there is a better pattern for this.

Anyway, I'll make the change early this week, and you can try it out if you want. We can iterate on the solution if it doesn't meet your needs.

Jean-PhilippeD commented 3 years ago

great, I'll test it as soon as possible.

Jean-PhilippeD commented 3 years ago

Hey I just had an other needs, what about if user is filling a password input like su - For now, the input is visible, what do you think about adding a props like inputType which maybe password type, and in that case, it should be hidden.

But maybe for that, you need to export the input field as read only to the parent component so the parent can verify what did the user sent to the Terminal ?

I'm not sure about that... my problem has just occured now so it's fresh in my mind...

jonmbake commented 3 years ago

For now, the input is visible, what do you think about adding a props like inputType which maybe password type, and in that case, it should be hidden.

Feel free to open up a separate issue for this. I think it makes sense to allow more control over how the input is displayed in general. We'll have to put more thought into it.

jonmbake commented 3 years ago

@Jean-PhilippeD You can try this out if you upgrade to v0.1.11.

Jean-PhilippeD commented 3 years ago

I just tested it and I got a problem, maybe the one you were talking about Symbol. edit: No it's just the react way..

If I modify the startingInputValue with setStartingInputValuein my component, it's working the first time, but once I changed it (with blank in my ctrl+C example), the next ctrl+C doesn't change anymore the startingInputValue so the trick doesn't work anymore.

I tried to set to false immediatly after the blank but your Terminal reject a false value (it tries to trim on false value)... by the way it's not really pretty to do:

setStartingInputValue ('')
setInputValue(false)

I have to take a break, I'll think on how I could manage this, maybe you'll have an idea.

Thanks for the support, I appreciate ;)

Jean-PhilippeD commented 3 years ago
            setInputValue('^C')
            setInputValue('')

works and seems to be acceptable for me

But what about exposing the same kind of features but instead of replacing the content, just appending ? Now I can "ctrl+c", I tried "ctrl+v" to paste content, it works but the input is replaced, while I could just want to append at the of the line ?

jonmbake commented 3 years ago

I think a better pattern for this might be something like an InputController, which could have an API like:

class InputController
  + triggerInputChange(String input)
  + onInputChange(String input)
  + onInputReceived(String input) 
jonmbake commented 3 years ago

Moved the remaining work for this to a new issue: https://github.com/jonmbake/react-terminal-ui/issues/13