Skip to content

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

Closed
aduth wants to merge 4 commits intomasterfrom
update/inline-link-ui-hooks-refactor
Closed

Format Library: Refactor inline link component to use React hooks#19487
aduth wants to merge 4 commits intomasterfrom
update/inline-link-ui-hooks-refactor

Conversation

@aduth
Copy link
Copy Markdown
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 aduth added [Type] Code Quality Issues or PRs that relate to code quality [Package] Format library /packages/format-library labels Jan 7, 2020
@aduth aduth mentioned this pull request Jan 7, 2020
Closed
@aduth
Copy link
Copy Markdown
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.

@aduth aduth mentioned this pull request Jan 8, 2020
Merged
6 tasks
@ellatrix
Copy link
Copy Markdown
Member

ellatrix commented Jan 9, 2020

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

@ellatrix
Copy link
Copy Markdown
Member

ellatrix commented Jan 9, 2020

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

@aduth
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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

Reviewers

@daniloercoli daniloercoli Awaiting requested review from daniloercoli

@ellatrix ellatrix Awaiting requested review from ellatrix

@etoledom etoledom Awaiting requested review from etoledom

@jorgefilipecosta jorgefilipecosta Awaiting requested review from jorgefilipecosta

@SergioEstevao SergioEstevao Awaiting requested review from SergioEstevao

Assignees

No one assigned

Labels

[Package] Format library /packages/format-library [Type] Code Quality Issues or PRs that relate to code quality

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@aduth @ellatrix