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

[refacto] Introduce stateless TextInputV2 #5013

Merged
merged 8 commits into from
Apr 22, 2024
Merged

Conversation

ijreilly
Copy link
Collaborator

@ijreilly ijreilly commented Apr 17, 2024

Context

As discussed with @lucasbordeau and @charlesBochet we are looking at making low level UI components stateless when possible.
Therefore TextInput should not handle a hotkey state. Instead hotkeys should be defined in the parent component (as done here in CreateProfile).

Introducing here TextInputV2 that is stateless and that can already replace TextInput without any behaviour change everywhere it is used with disableHotkey prop.

How was it tested?

Locally + Storybook

@ijreilly ijreilly marked this pull request as ready for review April 17, 2024 15:07
@ijreilly ijreilly self-assigned this Apr 18, 2024
@@ -1,141 +1,26 @@
import {
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't we keep just TextInputV2 ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You mean get rid of TextInput altogether now?
I want to take more time before replacing them where the hotkey scopes are not disabled to make sure I am dealing with them correctly

Copy link
Contributor

Choose a reason for hiding this comment

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

ok

@lucasbordeau lucasbordeau merged commit 68662fa into main Apr 22, 2024
8 of 9 checks passed
@lucasbordeau lucasbordeau deleted the refacto-text-input-v2 branch April 22, 2024 09:19
arnavsaxena17 pushed a commit to arnavsaxena17/twenty that referenced this pull request Oct 6, 2024
## Context
As discussed with @lucasbordeau and @charlesBochet we are looking at
making low level UI components stateless when possible.
Therefore TextInput should not handle a hotkey state. Instead hotkeys
should be defined in the parent component (as done here in
CreateProfile).

Introducing here TextInputV2 that is stateless and that can already
replace TextInput without any behaviour change everywhere it is used
with `disableHotkey` prop.

## How was it tested?
Locally + Storybook

---------

Co-authored-by: Lucas Bordeau <[email protected]>
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.

2 participants