search

idyll-lang / idyll

Create explorable explanations and interactive essays.
http://idyll-lang.org/
MIT License
2.01k stars 87 forks source link

Invalid markup syntax will lead to crash #675

Open huozhi opened 3 years ago

huozhi commented 3 years ago

Describe the bug A clear and concise description of what the bug is.

To Reproduce Steps to reproduce the behavior:

  1. Go to homepage https://idyll-lang.org/
  2. Click on the editable area and start to input []()
  3. See error

Expected behavior render []() as text. or maybe just warn user it's not valid

Screenshots N/A

Desktop (please complete the following information):

Additional context N/A

mathisonian commented 3 years ago

Thanks for reporting this @huozhi. We should definitely handle this more gracefully, and not have it blow up the entire website.

If anyone is willing to contribute a patch it would be appreciated! The relevant code for the homepage is here: https://github.com/idyll-lang/idyll/blob/master/packages/idyll-docs/pages/index.js#L140-L145.

mrviniciux commented 3 years ago

@mathisonian hello there, I'm checking this issue on component text-input.js and I see that there's no validation in onChange event, just a this.props.updateProps that I really don't know what it does exactly. But I guess this seems to be normal, since the input will be used in many different situations and patterns right?

I'm new in the idyll universe, so maybe my question here is trivial... Is there any kind of "regex" that matches the input from "Write your own equation"?

Or this must be done only in the "idyll-docs" and not in "idyll-components"? Because my initial thought was to put some kind of red label above the input indicating error, very similar to material design form error highlight.

mathisonian commented 3 years ago

Hi @mrviniciux - would you mind clarifying the use case a little bit?

With the [TextInput /] component, an author would typically use it by binding a variable to its value property, so that each time a user writes text, it updates the value of the variable:

[var name:"myvar" value:"" /] // initialize a variable as the empty string

[TextInput value:myvar /]

The value of myvar is [Display value:myvar /]`

which would result in this behavior:

https://user-images.githubusercontent.com/1074773/115473005-c20fc080-a1ef-11eb-9888-a62cc32385ee.mov

Looking at the implementation of text-input, you can see that the updateProps function is the how the component can tell Idyll, "If a variable is bound to my value property, set it to this new value", by calling `updateProps({ value: "some new value" }). Note that any component has access to this function, it is the mechanism by which any component is expected to effect change to the state of an Idyll article. There's a little more on that here.

mrviniciux commented 3 years ago

@mathisonian sorry for the delay on this, finally I take some time to get back on board.

I opened a PR as mentioned above and hope that helps. In the next weeks I will stay tuned for your feedback and hope we can do something great about this. See ya!