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

Make text component editable in the canvas #1694

Merged
merged 7 commits into from
Feb 22, 2023
Merged

Conversation

Janpot
Copy link
Member

@Janpot Janpot commented Feb 21, 2023

Make text component editable after double clicking the selected text node.

  • Only when mode is set to "text". markdown requires more elaborate implementation.
  • Not polished, the response isn't always great
  • Property in dom only updates after the text is deselcted
Screen.Recording.2023-02-21.at.17.59.51.mov

@oliviertassinari oliviertassinari requested a deployment to editabl-experiment - toolpad-db PR #1694 February 21, 2023 17:02 — with Render Abandoned
@Janpot Janpot added the core Infrastructure work going on behind the scenes label Feb 21, 2023
@oliviertassinari oliviertassinari temporarily deployed to editabl-experiment - toolpad PR #1694 February 21, 2023 17:04 — with Render Destroyed
@apedroferreira
Copy link
Member

apedroferreira commented Feb 21, 2023

Nice, this is a useful change. Are we just looking to merge this quick or to polish it more?
Some things I expected to be different when I tried this was:

  • Single clicking should start editing immediately instead of double clicking if the cursor is a text cursor
  • Should pressing "Enter" blur the input or add a newline as it does right now? Otherwise to leave "editing mode" we have to click away (which might be ok, just not sure)
  • Not exactly related with this PR, but right now when we place a Text component it has no value so its starts practically invisible. Should Text components have a default value like buttons do, for example?

@Janpot
Copy link
Member Author

Janpot commented Feb 21, 2023

  • Single clicking should start editing immediately instead of double clicking if the cursor is a text cursor

Yep, that's what I mean with polish. I didn't spend too much time on UX. Just wanted to see how hard it is to build this kind of functionality within current Toolpad. The answer is, it doesn't seem to be too complicated for simple text.

  • Should pressing "Enter" blur the input or add a newline as it does right now? Otherwise to leave "editing mode" we have to click away (which might be ok, just not sure)

Not sure what it should do for text. Maybe we should replace newlines with <br>? It should add a newline for markdown.

  • Not exactly related with this PR, but right now when we place a Text component it has no value so its starts practically invisible. Should Text components have a default value like buttons do, for example?

I believe it should yes, not sure when that regressed, but it must have been a long time ago. Ideally It starts with a placeholder text, in editing mode, with all text selected. So that you can immediately start typing. Not sure how hard this would be to implement.

@apedroferreira
Copy link
Member

apedroferreira commented Feb 21, 2023

Not sure what it should do for text. Maybe we should replace newlines with
? It should add a newline for markdown.

Was just seeing what you thought, but yeah I think the current behavior should be ok too. Not sure if there's advantages to adding a <br/> tag, I guess newline should be fine? And it's good if the normal text works in a consistent way with how markdown works / will work.

I believe it should yes, not sure when that regressed, but it must have been a long time ago. Ideally It starts with a placeholder text, in editing mode, with all text selected. So that you can immediately start typing. Not sure how hard this would be to implement.

Yeah that sounds great! I think it shouldn't be too hard, it would probably just require adding/changing some logic in the component like you already did in this PR?

@oliviertassinari oliviertassinari temporarily deployed to editabl-experiment - toolpad PR #1694 February 22, 2023 10:16 — with Render Destroyed
@Janpot
Copy link
Member Author

Janpot commented Feb 22, 2023

I gave it a bit more polish

  • add default value
  • disallow Enter to create newlines
  • autofocus and handle cursor position correctly
  • show all whitespace
Screen.Recording.2023-02-22.at.11.58.58.mov

@Janpot Janpot changed the title Quick experiment making Text input editable in-canvas Make text component editable in the canvas Feb 22, 2023
@oliviertassinari oliviertassinari temporarily deployed to editabl-experiment - toolpad PR #1694 February 22, 2023 10:55 — with Render Destroyed
@oliviertassinari oliviertassinari temporarily deployed to editabl-experiment - toolpad PR #1694 February 22, 2023 11:02 — with Render Destroyed
Copy link
Member

@bharatkashyap bharatkashyap left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great change! Looks good to me

@Janpot Janpot merged commit aa5eccb into master Feb 22, 2023
@Janpot Janpot deleted the editabl-experiment branch February 22, 2023 14:10
@Janpot Janpot mentioned this pull request Feb 22, 2023
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Infrastructure work going on behind the scenes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants