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

[Feature] Popover when setting Link #1

Open
wants to merge 14 commits into
base: main
Choose a base branch
from

Conversation

CorreyL
Copy link
Owner

@CorreyL CorreyL commented Jan 29, 2024

As a small personal coding challenge, I expanded upon the idea suggested here:

AmruthPillai#1727 (comment)

This PR replaces the window.prompt for adding/editing a link in the Text Editor with a Popover component

Video.mp4

The details in this PR will be expanded upon if the Reactive Resume maintainers have any interest in this feature being added to the main codebase.

Coding timeblock exceeded, stopping for now to resume another day
Fixes bug where the Popover would not close when adding a link to
previously selected text
Replace the original text, in-case the user inputted new text
Setting `to` to displayText.length only works if `from === 1` (i.e. the
cursor is at the beginning of the textbox)

The correct calculation is to offset from `from`, then add
`displayText.length` to it
Saves the user presses on the keyboard from having to type the protocol,
while leaving it editable on the off-chance the user wishes to change it
to http://, or some other protocol
Original bug fixed, new bug found
If the cursor was in the middle of text with a hyperlink, or only part
of the text with a hyperlink was selected, then the displayText would
not accurately reflect the full text that the link was associated with

Since this refactor required accessing and modifying the editor, pass
the editor as a prop instead, and instantiate what were previously props
as variables within the InsertLinkForm component
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.

1 participant