Paths

Table of Contentst

-
lib/shared/
-
shared/
2/4
typeahead-utils.js
-
web/
-
chat/
2/4
chat-input-bar.react.js
2/4
typeahead-tooltip.react.js
-
utils/
2/4
typeahead-utils.js

[web] Keyboard support for typeahead [6/13] - Get rid of entire text and rename matched strings
ClosedPublic
Actions

Authored by • przemek on Dec 27 2022, 4:49 AM.

Details

Reviewers

inka
kamil
ginsu
tomek

Commits

rCOMM6994060d445e: [web] Keyboard support for typeahead [6/13] - Get rid of entire text and rename…

Summary

Some renaming of arguments and variables.
Removed entireText from TypeaheadMatchedStrings as we only needed its length in getTypeaheadTooltipActions function.
Length of entire text can be easily obtained by textBeforeAtSymbol.length + usernamePrefix.length + 1 where 1 stands for @ symbol.

Test Plan

Typeahead without keyboard support works.
Final tests performed in last diffs.

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

• przemek created this revision.Dec 27 2022, 4:49 AM

Herald added subscribers: atul, ashoat. · View Herald TranscriptDec 27 2022, 4:49 AM

Harbormaster completed remote builds in B14779: Diff 20157.Dec 27 2022, 5:03 AM

• przemek requested review of this revision.Dec 27 2022, 5:03 AM

• przemek added a parent revision: D6045: [web] Keyboard support for typeahead [5/13] - Moved finding suggested users out of tooltip component.Dec 27 2022, 5:31 AM

• przemek added a child revision: D6047: [web] Keyboard support for typeahead [7/13] - Refactor getTypeaheadTooltipActions using params as argument.

inka accepted this revision.Dec 28 2022, 5:58 AM

inka added a reviewer: tomek.

tomek accepted this revision.Dec 30 2022, 8:43 AM

This revision is now accepted and ready to land.Dec 30 2022, 8:43 AM

Closed by commit rCOMM6994060d445e: [web] Keyboard support for typeahead [6/13] - Get rid of entire text and rename… (authored by • przemek). · Explain WhyJan 12 2023, 6:21 AM

This revision was automatically updated to reflect the committed changes.

• przemek added a commit: rCOMM6994060d445e: [web] Keyboard support for typeahead [6/13] - Get rid of entire text and rename….

Revision Contents
Changeset List

Path

Size

lib/

shared/

typeahead-utils.js

8 lines

web/

chat/

chat-input-bar.react.js

2 lines

typeahead-tooltip.react.js

27 lines

utils/

typeahead-utils.js

15 lines

Diff 20864

View Options

lib/shared/typeahead-utils.js

This file was added.

				div.containerViewerLiked {
				padding: 4px;
				border-radius: 4px;
				background-color: var(--shades-black-80);
				}

				div.container:hover {
				cursor: pointer;
				}
				tomekUnsubmitted Not Done Inline Actions For other tooltip actions we don't need to manually set this style. Can we use the mechanism we already have, or are there any reasons for not doing so? tomek: For other tooltip actions we don't need to manually set this style. Can we use the mechanism we…
				ginsuAuthorUnsubmitted Done Inline Actions I believe the other icons don't need to set this style because it is built into `IcomoonReact`. When I switched the Like action tooltip button with one of the icons the hover style just works out of the box. Wondering if @atul has more context since he is pretty familiar with the icons stuff ginsu: I believe the other icons don't need to set this style because it is built into `IcomoonReact`.
				tomekUnsubmitted Not Done Inline Actions It wouldn't make too much sense for `Icomoon` to provide hover effects for icons because they could be used in other places - not only as buttons. It seems like the style on hover is applied in `message-tooltip.css` div.messageActionButtons svg:hover { cursor: pointer; color: var(--fg); } so it should be enough to apply this style also to the new button. tomek: It wouldn't make too much sense for `Icomoon` to provide hover effects for icons because they…
				ginsuAuthorUnsubmitted Done Inline Actions Ahhhhh that makes sense, thank you for clarifying, I will make the necessary changes ginsu: Ahhhhh that makes sense, thank you for clarifying, I will make the necessary changes

View Options

web/chat/chat-input-bar.react.js

This file was added.

				div.containerViewerLiked {
				padding: 4px;
				border-radius: 4px;
				background-color: var(--shades-black-80);
				}

				div.container:hover {
				cursor: pointer;
				}
				tomekUnsubmitted Not Done Inline Actions For other tooltip actions we don't need to manually set this style. Can we use the mechanism we already have, or are there any reasons for not doing so? tomek: For other tooltip actions we don't need to manually set this style. Can we use the mechanism we…
				ginsuAuthorUnsubmitted Done Inline Actions I believe the other icons don't need to set this style because it is built into `IcomoonReact`. When I switched the Like action tooltip button with one of the icons the hover style just works out of the box. Wondering if @atul has more context since he is pretty familiar with the icons stuff ginsu: I believe the other icons don't need to set this style because it is built into `IcomoonReact`.
				tomekUnsubmitted Not Done Inline Actions It wouldn't make too much sense for `Icomoon` to provide hover effects for icons because they could be used in other places - not only as buttons. It seems like the style on hover is applied in `message-tooltip.css` div.messageActionButtons svg:hover { cursor: pointer; color: var(--fg); } so it should be enough to apply this style also to the new button. tomek: It wouldn't make too much sense for `Icomoon` to provide hover effects for icons because they…
				ginsuAuthorUnsubmitted Done Inline Actions Ahhhhh that makes sense, thank you for clarifying, I will make the necessary changes ginsu: Ahhhhh that makes sense, thank you for clarifying, I will make the necessary changes

View Options

web/chat/typeahead-tooltip.react.js

This file was added.

				div.containerViewerLiked {
				padding: 4px;
				border-radius: 4px;
				background-color: var(--shades-black-80);
				}

				div.container:hover {
				cursor: pointer;
				}
				tomekUnsubmitted Not Done Inline Actions For other tooltip actions we don't need to manually set this style. Can we use the mechanism we already have, or are there any reasons for not doing so? tomek: For other tooltip actions we don't need to manually set this style. Can we use the mechanism we…
				ginsuAuthorUnsubmitted Done Inline Actions I believe the other icons don't need to set this style because it is built into `IcomoonReact`. When I switched the Like action tooltip button with one of the icons the hover style just works out of the box. Wondering if @atul has more context since he is pretty familiar with the icons stuff ginsu: I believe the other icons don't need to set this style because it is built into `IcomoonReact`.
				tomekUnsubmitted Not Done Inline Actions It wouldn't make too much sense for `Icomoon` to provide hover effects for icons because they could be used in other places - not only as buttons. It seems like the style on hover is applied in `message-tooltip.css` div.messageActionButtons svg:hover { cursor: pointer; color: var(--fg); } so it should be enough to apply this style also to the new button. tomek: It wouldn't make too much sense for `Icomoon` to provide hover effects for icons because they…
				ginsuAuthorUnsubmitted Done Inline Actions Ahhhhh that makes sense, thank you for clarifying, I will make the necessary changes ginsu: Ahhhhh that makes sense, thank you for clarifying, I will make the necessary changes

View Options

web/utils/typeahead-utils.js

This file was added.

				div.containerViewerLiked {
				padding: 4px;
				border-radius: 4px;
				background-color: var(--shades-black-80);
				}

				div.container:hover {
				cursor: pointer;
				}
				tomekUnsubmitted Not Done Inline Actions For other tooltip actions we don't need to manually set this style. Can we use the mechanism we already have, or are there any reasons for not doing so? tomek: For other tooltip actions we don't need to manually set this style. Can we use the mechanism we…
				ginsuAuthorUnsubmitted Done Inline Actions I believe the other icons don't need to set this style because it is built into `IcomoonReact`. When I switched the Like action tooltip button with one of the icons the hover style just works out of the box. Wondering if @atul has more context since he is pretty familiar with the icons stuff ginsu: I believe the other icons don't need to set this style because it is built into `IcomoonReact`.
				tomekUnsubmitted Not Done Inline Actions It wouldn't make too much sense for `Icomoon` to provide hover effects for icons because they could be used in other places - not only as buttons. It seems like the style on hover is applied in `message-tooltip.css` div.messageActionButtons svg:hover { cursor: pointer; color: var(--fg); } so it should be enough to apply this style also to the new button. tomek: It wouldn't make too much sense for `Icomoon` to provide hover effects for icons because they…
				ginsuAuthorUnsubmitted Done Inline Actions Ahhhhh that makes sense, thank you for clarifying, I will make the necessary changes ginsu: Ahhhhh that makes sense, thank you for clarifying, I will make the necessary changes

[web] Keyboard support for typeahead [6/13] - Get rid of entire text and rename matched stringsClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 20864

lib/shared/typeahead-utils.js

web/chat/chat-input-bar.react.js

web/chat/typeahead-tooltip.react.js

web/utils/typeahead-utils.js

[web] Keyboard support for typeahead [6/13] - Get rid of entire text and rename matched strings
ClosedPublic
Actions

Revision Contents
Changeset List