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

Fix formatBar not hidden after highlight and backspacing some text #4269

Merged
merged 3 commits into from
Mar 26, 2020
Merged

Fix formatBar not hidden after highlight and backspacing some text #4269

merged 3 commits into from
Mar 26, 2020

Conversation

ThobyV
Copy link
Contributor

@ThobyV ThobyV commented Mar 24, 2020 •
edited
Loading

This fixes element-hq/element-web#12671

Hopeful for a review soon. Seems to be that _onSelectionChange does not trigger on backspace so catching this event/key manually was quiet useful.

Signed-off-by: thobyv-kismat vivee18@gmail.com
@turt2live turt2live requested a review from a team March 24, 2020 20:04
bwindels
bwindels suggested changes Mar 25, 2020
View reviewed changes
src/components/views/rooms/BasicMessageComposer.js Outdated
} else if (event.key === Key.TAB) {
this._tabCompleteName();
handled = true;
} else if (event.key === Key.BACKSPACE) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice, thanks for this! Could you add DELETE as well please?
Copy link
Contributor

@bwindels bwindels Mar 25, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

(Which will need to be added to Keyboard.ts: DELETE: "Delete")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh! Ok thanks for the review @bwindels I'll make those changes asap.
bwindels
bwindels approved these changes Mar 26, 2020
View reviewed changes
Copy link
Contributor

@bwindels bwindels left a comment •
edited
Loading

Choose a reason for hiding this comment

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

lgtm, apart from typo to repeat event.key (fixed)
@bwindels
Copy link
Contributor

@ThobyV Could you rebase your branch to latest develop please? The build for the end-to-end tests is failing because of some recent typescript changes after you branched off.
@ThobyV
Copy link
Contributor Author

ThobyV commented Mar 26, 2020

Sure @bwindels I'll rebase asap and commit your suggestions. Thanks alot for the review. ☺
@bwindels
Copy link
Contributor

Sure @bwindels I'll rebase asap and commit your suggestions. Thanks alot for the review. ☺

Thanks, already committed my own suggestion btw :)
@ThobyV ThobyV force-pushed the thobyv/hide-formatBar-on-highlight-backspacing-message branch from 3a1428e to 2bd112c Compare March 26, 2020 12:04
@ThobyV
Copy link
Contributor Author

ThobyV commented Mar 26, 2020

@bwindels I am so sorry I made a mistake after rebasing - I unknowingly force pushed the updates and that seems to have removed your commit. I am quiet inexperienced with rebasing. Please accept my apology.
@bwindels
Copy link
Contributor

@bwindels I am so sorry I made a mistake after rebasing - I unknowingly force pushed the updates and that seems to have removed your commit. I am quiet inexperienced with rebasing. Please accept my apology.

No worries, I should have said "merge in develop", as rebasing indeed leaves you no other choice but to force push :)

In any case, changes look good and build is working, so merging
@bwindels bwindels merged commit 053a5a5 into matrix-org:develop Mar 26, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

2 participants

@ThobyV @bwindels