Paths

Table of Contentst

[web] Fix app switcher for thread creation
AbandonedPublic
Actions

Authored by • jacek on Jul 7 2022, 5:34 AM.

Details

Reviewers

tomek
atul
• abosh

Summary

Currently app switcher highlights current app (on the left side) using navTabSelecter. The change allows highlighting chat icon in both chat and chat-creation navigation state.

After (in chat creation state):

Screenshot_Google Chrome_2022-07-07_150114.png (200×249 px, 6 KB)

Test Plan

Enter chat creation mode. The chat icon on the left should remain highlighted.

Diff Detail

Repository

rCOMM Comm

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

• jacek created this revision.Jul 7 2022, 5:34 AM

Herald added subscribers: • abosh, • adrian, ashoat. · View Herald TranscriptJul 7 2022, 5:34 AM

Harbormaster completed remote builds in B10347: Diff 14255.Jul 7 2022, 5:49 AM

• jacek requested review of this revision.Jul 7 2022, 5:49 AM

• jacek added a parent revision: D4457: [web] Introduce `chat-creation` mode in `navInfo`.Jul 7 2022, 5:57 AM

• jacek added a child revision: D4465: [web] Support chat creation in `ChatMessageListContainer`.

• jacek edited the summary of this revision. (Show Details)Jul 7 2022, 6:01 AM

• abosh requested changes to this revision.Jul 7 2022, 7:00 AM

• abosh added inline comments.

web/selectors/nav-selectors.js
17 ↗	(On Diff #14255)	Why was this change made? Flow/ESLint wasn't complaining and it doesn't match the syntax in the rest of the imports.
218 ↗	(On Diff #14255)	This is a bit pedantic, but looks cleaner. Not necessary to implement, though.

This revision now requires changes to proceed.Jul 7 2022, 7:00 AM

• abosh added inline comments.Jul 7 2022, 7:45 AM

web/selectors/nav-selectors.js
218–222 ↗	(On Diff #14255)	Actually, in a follow up to my above comment, this can also be shortened with a ternary.

• jacek added inline comments.Jul 8 2022, 4:16 AM

web/selectors/nav-selectors.js
17 ↗	(On Diff #14255)	Interesting, I really don't know. Probably somehow by accident. I'll revert it.
218 ↗	(On Diff #14255)	Not sure which is better, It could be a bit dangerous, because if we introduce some new tab starting with `chat` in the future, this check would automatically cover it, what may not be the best solution.

• abosh accepted this revision.Jul 8 2022, 7:17 AM

• abosh added inline comments.

web/selectors/nav-selectors.js
218 ↗	(On Diff #14255)	That's a great point. In that case, I don't think it's necessary to incorporate this suggestion.

This revision is now accepted and ready to land.Jul 8 2022, 7:17 AM

accidentally hit accept, sorry. Will accept once this change is reverted.

This revision now requires changes to proceed.Jul 8 2022, 7:18 AM

• jacek planned changes to this revision.Jul 11 2022, 1:58 AM

revert accidental import change

Harbormaster completed remote builds in B10441: Diff 14385.Jul 11 2022, 2:45 AM

Abandoning due to changes in previous diffs that eliminated the need for the change here.

Revision Contents
Changeset List

Path

Size

web/

selectors/

nav-selectors.js

4 lines

Diff 14385

View Options

web/selectors/nav-selectors.js

[web] Fix app switcher for thread creationAbandonedPublicActions