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

Update EditInPlace to use Antd components #4493

Merged
merged 4 commits into from
Dec 26, 2019
Merged

Conversation

gabrieldutra
Copy link
Member

@gabrieldutra gabrieldutra commented Dec 26, 2019

What type of PR is this? (check all applicable)

  • Refactor

Description

This updates EditInPlace to use Antd components instead of rd-form.

I considered using Antd's Typography, but it's not 100% the same thing and Typography doesn't seem to support multi-line with Shift+enter or Textarea as editor.

Anyway, most of the existing EditInPlace's didn't change, aside from the titles that now have a smaller font-size when being edited:

Before
before

After
after

Also took the opportunity to allow className in the component.

@gabrieldutra gabrieldutra self-assigned this Dec 26, 2019
Copy link
Collaborator

@kravets-levko kravets-levko left a comment

Choose a reason for hiding this comment

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

Everything looks good, just one this I'd like to change (see my comment).

Also - WDYT about rewriting this component to function-based? Will it make code anyhow better/cleaner? 🤔

@gabrieldutra
Copy link
Member Author

WDYT about rewriting this component to function-based? Will it make code anyhow better/cleaner?

I don't think it's worth it, the component looks simple already 🙂

Copy link
Collaborator

@kravets-levko kravets-levko left a comment

Choose a reason for hiding this comment

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

👍

@gabrieldutra gabrieldutra merged commit fd46194 into master Dec 26, 2019
@gabrieldutra gabrieldutra deleted the update-edit-in-place branch December 26, 2019 15:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants