Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

New composer: show format bar on selection #3386

Merged
merged 27 commits into from
Sep 6, 2019
Merged

Conversation

bwindels
Copy link
Contributor

@bwindels bwindels commented Sep 4, 2019

so we can return a DocumentOffset from there without breakage
we need this to get both offsets of the selection boundaries

getSelectionOffsetAndText offers the extra flexibility,
getCaretOffsetAndText keeps the old api for focusNode/focusOffset

Also did some renaming here now that it's not just for the caret anymore
we'll need this to start a range for the selection
we'll need this when replacing selection, to preserve
newlines, etc ... in the selected range (e.g. we can't just
use range.text).
@bwindels bwindels force-pushed the bwindels/cider-formatbar branch from 0ce75c2 to 322775e Compare September 4, 2019 14:37
also rename caret away as this isn't used for the caret solely anymore
this follows the documentation of {focus|anchor}{Offset|Node} better
and was causing problems for creating ranges from the document selection
when doing ctrl+A in firefox as the anchorNode/Offset was expressed
as childNodes from the editor root.
if index is not found, it means the last position should be returned
if there is any.

We still return -1 for empty documents, as index should always point
to a valid part if positive.
@bwindels bwindels force-pushed the bwindels/cider-formatbar branch from 322775e to af53598 Compare September 4, 2019 14:44
@bwindels bwindels requested review from a team and removed request for a team September 4, 2019 14:56
@bwindels
Copy link
Contributor Author

bwindels commented Sep 4, 2019

Removed review request because detecting the selection doesn't work in Chrome for some reason.

@bwindels bwindels requested a review from a team September 5, 2019 09:03
@turt2live turt2live requested review from turt2live and removed request for a team September 5, 2019 15:05
Copy link
Member

@turt2live turt2live left a comment

Choose a reason for hiding this comment

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

I think this is generally sane - I'm getting a bit concerned about how much math we have just to deal with text though.

src/editor/operations.js Outdated Show resolved Hide resolved
@bwindels
Copy link
Contributor Author

bwindels commented Sep 6, 2019

Thanks for the review!

I think this is generally sane - I'm getting a bit concerned about how much math we have just to deal with text though.

You mean the code inside src/editor/ or in the BasicMessageComposer component?

@bwindels bwindels merged commit 02681d5 into develop Sep 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants