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

Format Library: Refactor inline link component to use React hooks #19487

Closed
wants to merge 4 commits into from

Conversation

aduth
Copy link
Member

@aduth aduth commented Jan 7, 2020

Related: #19440

This pull request seeks to refactor the inline link UI component (used for adding, viewing, and editing links in a RichText field) as a function component. Originally this was planned as a blocking change to allow for the implementation of additional custom React hooks to be used in this component as a resolution of #18755. This is planned to be addressed separately, and may ultimately not use hooks.

The changes here are intended to be a one-to-one port of the existing component, with the exception of the revisions mentioned at #19440 (comment). I may plan to extract this (6523f57) as a separate pull request in order to simplify review.

Testing Instructions:

Verify there are no regressions in viewing, adding, editing, or removing a link.

@aduth
Copy link
Member Author

aduth commented Jan 7, 2020

Originally this was planned as a blocking change to allow for the implementation of additional custom React hooks to be used in this component as a resolution of #18755. This is planned to be addressed separately, and may ultimately not use hooks.

See also: #19488, which is currently set to merge to this branch.

@ellatrix
Copy link
Member

ellatrix commented Jan 9, 2020

Looks like this will conflict with #19462. Any preference on what to merge first?

@ellatrix
Copy link
Member

ellatrix commented Jan 9, 2020

Ah, it looks like that PR is also rewriting the component with hooks.

@aduth
Copy link
Member Author

aduth commented Jan 9, 2020

Ah, it looks like that PR is also rewriting the component with hooks.

Oof 😄

(cc @youknowriad, I haven't looked closely enough to have a good sense on how to proceed)

@aduth
Copy link
Member Author

aduth commented Jan 9, 2020

I guess #19462 simplifies the implementation more, in that LinkControl handles most of the specific behaviors? I'd want to consider how this would impact #19488, but I suppose if it can be implemented at LinkControl instead, it would have the benefit of being more universal.

@aduth
Copy link
Member Author

aduth commented Jan 16, 2020

I think it's probably not worth continuing with this if the plan is to use LinkControl more consistently throughout the UI, like iterated in #19462.

@aduth aduth closed this Jan 16, 2020
@aristath aristath deleted the update/inline-link-ui-hooks-refactor branch November 10, 2020 14:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Format library /packages/format-library [Type] Code Quality Issues or PRs that relate to code quality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants