-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Conversation List Highlighting #7453
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
base: main
Are you sure you want to change the base?
Changes from 59 commits
516c650
5e47c5c
cb57288
d781d40
9489563
d76782c
155ca30
9ce876d
3352a3d
46f000f
2dfcf8c
28d12a6
5316376
1cd1527
fda89ea
369a2d4
62f6462
7e06391
7658722
8819e4d
1e5c0ca
283247c
2b00d58
ab03113
54fc9e8
297e444
04c4a8b
811bde2
91ebd7a
d0e8a28
cd98e1d
3573884
c2be1f1
76de8cb
5566c94
a771c29
4272f5f
cfcd9d5
7bf9b78
a018c70
9543b81
a4407ce
b782aed
5d584f6
d6cd88f
f71fb14
21cc370
bcfe4da
770953f
1054177
5b77a52
6588b5b
37cc148
0b9b262
e085483
0c0e037
4f0afcb
fadd5ba
3dd9a7a
5444447
966f668
90c1fe7
ba7d149
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -290,7 +290,12 @@ export const BaseConversationListItem: FunctionComponent<PropsType> = | |
| data-testid={testId} | ||
| disabled={disabled} | ||
| onClick={onClick} | ||
| onMouseDown={onMouseDown} | ||
| onMouseDown={event => { | ||
| event.preventDefault(); | ||
|
||
| if (onMouseDown) { | ||
| onMouseDown(); | ||
| } | ||
| }} | ||
| type="button" | ||
| > | ||
| {contents} | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,99 @@ | ||
| // Copyright 2024 Signal Messenger, LLC | ||
|
||
| // SPDX-License-Identifier: AGPL-3.0-only | ||
|
|
||
| import { expect } from 'playwright/test'; | ||
|
|
||
| import { Bootstrap } from '../bootstrap'; | ||
| import type { App } from '../playwright'; | ||
| import * as durations from '../../util/durations'; | ||
| import { strictAssert } from '../../util/assert'; | ||
|
|
||
| describe('conversation list highlighting', function (this: Mocha.Suite) { | ||
| this.timeout(durations.MINUTE); | ||
|
|
||
| let bootstrap: Bootstrap; | ||
| let app: App; | ||
|
|
||
| beforeEach(async () => { | ||
| bootstrap = new Bootstrap(); | ||
| await bootstrap.init(); | ||
| app = await bootstrap.link(); | ||
| }); | ||
|
|
||
| afterEach(async function (this: Mocha.Context) { | ||
| if (!bootstrap) { | ||
| return; | ||
| } | ||
|
|
||
| await bootstrap.maybeSaveLogs(this.currentTest, app); | ||
| await app.close(); | ||
| await bootstrap.teardown(); | ||
| }); | ||
|
|
||
| it('highlights only on hover', async () => { | ||
| const { contacts, desktop } = bootstrap; | ||
| const [friend] = contacts; | ||
|
|
||
| await friend.sendText(desktop, 'hi'); | ||
|
|
||
| const page = await app.getWindow(); | ||
| const leftPane = page.locator('#LeftPane'); | ||
| const item = leftPane | ||
| .locator('.module-conversation-list__item--contact-or-conversation') | ||
| .first(); | ||
|
|
||
| await item.waitFor(); | ||
|
|
||
| const defaultBg = await item.evaluate( | ||
| el => getComputedStyle(el).backgroundColor | ||
| ); | ||
|
|
||
| await item.hover(); | ||
| const hoverBg = await item.evaluate( | ||
| el => getComputedStyle(el).backgroundColor | ||
| ); | ||
| expect(hoverBg).not.toBe(defaultBg); | ||
|
|
||
| await page.mouse.move(0, 0); | ||
| const afterHoverBg = await item.evaluate( | ||
| el => getComputedStyle(el).backgroundColor | ||
| ); | ||
| expect(afterHoverBg).toBe(defaultBg); | ||
| }); | ||
|
|
||
| it('does not stay highlighted after drag', async () => { | ||
| const { contacts, desktop } = bootstrap; | ||
| const [friend] = contacts; | ||
|
|
||
| await friend.sendText(desktop, 'hi'); | ||
|
|
||
| const page = await app.getWindow(); | ||
| const leftPane = page.locator('#LeftPane'); | ||
| const item = leftPane | ||
| .locator('.module-conversation-list__item--contact-or-conversation') | ||
| .first(); | ||
|
|
||
| await item.waitFor(); | ||
|
|
||
| const defaultBg = await item.evaluate( | ||
| el => getComputedStyle(el).backgroundColor | ||
| ); | ||
|
|
||
| const box = await item.boundingBox(); | ||
| strictAssert(box, 'Bounding box not found'); | ||
|
|
||
| await page.mouse.move(box.x + box.width / 2, box.y + box.height / 2); | ||
| await page.mouse.down(); | ||
| await page.mouse.move(box.x + box.width / 2, box.y - 40); | ||
| await page.mouse.up(); | ||
| await page.mouse.move(0, 0); | ||
|
|
||
| const afterDragBg = await item.evaluate( | ||
| el => getComputedStyle(el).backgroundColor | ||
| ); | ||
| expect(afterDragBg).toBe(defaultBg); | ||
|
|
||
| const isActive = await item.evaluate(el => document.activeElement === el); | ||
| expect(isActive).toBe(false); | ||
| }); | ||
| }); | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are we trying to set the colors to the opposite of the
hoverstate above (I may just be not understanding).Also, have you tested this in light mode? I'm not sure nested mixings work the way we always intend. We've been moving to the newer CSS syntax
light-dark()which might work well here.