-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
feat: Devtool for sticky events MSC4354 #32741
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
Changes from 4 commits
adcba32
335a5df
37f768d
424129a
15945ed
1a09bba
eabe6b2
2249c6a
0d458b7
3eef477
6a7a5d7
b43fb2a
6e41f5b
7f7cd1b
ad58b21
03638a7
ec460b2
63e49ac
a27cb71
4522f18
fa9039c
d0d00d4
b626701
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 | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,364 @@ | ||||||||||
| /* | ||||||||||
| * Copyright 2026 Element Creations Ltd. | ||||||||||
| * | ||||||||||
| * SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Commercial | ||||||||||
| * Please see LICENSE files in the repository root for full details. | ||||||||||
| */ | ||||||||||
|
|
||||||||||
| import React, { type ChangeEvent, useContext, useEffect, useMemo, useState } from "react"; | ||||||||||
| import { Pill } from "@element-hq/web-shared-components"; | ||||||||||
| import { MatrixEvent, type IContent, RoomStickyEventsEvent } from "matrix-js-sdk/src/matrix"; | ||||||||||
| import { Form, SettingsToggleInput } from "@vector-im/compound-web"; | ||||||||||
| import { v4 as uuidv4 } from "uuid"; | ||||||||||
| import { logger } from "nx/src/utils/logger"; | ||||||||||
|
|
||||||||||
| import BaseTool, { DevtoolsContext, type IDevtoolsProps } from "./BaseTool.tsx"; | ||||||||||
| import { _t, _td } from "../../../../languageHandler.tsx"; | ||||||||||
| import { | ||||||||||
| EventEditor, | ||||||||||
| eventTypeField, | ||||||||||
| EventViewer, | ||||||||||
| type IEditorProps, | ||||||||||
| stickyDurationField, | ||||||||||
| stringify, | ||||||||||
| } from "./Event.tsx"; | ||||||||||
| import Field from "../../elements/Field.tsx"; | ||||||||||
| import MatrixClientContext from "../../../../contexts/MatrixClientContext.tsx"; | ||||||||||
|
|
||||||||||
| /** | ||||||||||
| * Devtool to explore sticky events in the current room. | ||||||||||
| * It allows you to see all sticky events, filter them by type, and view their content. | ||||||||||
| * @param onBack | ||||||||||
| * @param setTool | ||||||||||
| * @constructor | ||||||||||
t3chguy marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||||||||||
| */ | ||||||||||
| export const StickyStateExplorer: React.FC<IDevtoolsProps> = ({ onBack, setTool }) => { | ||||||||||
| const context = useContext(DevtoolsContext); | ||||||||||
| const [eventType, setEventType] = useState<string | null>(null); | ||||||||||
| const [event, setEvent] = useState<MatrixEvent | null>(null); | ||||||||||
|
||||||||||
| const [eventType, setEventType] = useState<string | null>(null); | |
| const [event, setEvent] = useState<MatrixEvent | null>(null); | |
| const [eventType, setEventType] = useState<string>(); | |
| const [event, setEvent] = useState<MatrixEvent>(); |
and the type will be T | undefined
Outdated
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.
this should use useTypedEventEmitterState
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.
done here a27cb71
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's this async? Also probably ought to be useCallback().
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.
It is because BaseTool IProps expects onAction(this: void): Promise<string | void>
Outdated
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.
This state feels a bit redundant? Isn't it just a function of timeRemaining?
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.
right, done b626701
t3chguy marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
Outdated
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.
Should be using :hover selector in CSS rather than JS hover tracking. Also probably needs keyboard accessibility styling rather than just mouse pointer hover
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.
Done for hover.
I don't know what is keyboard accessibility styling? maybe it is using :focus css ? can you give me some advice to fix it?
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.
https://developer.mozilla.org/en-US/docs/Web/CSS/Reference/Selectors/:focus-visible - :focus also applies to cursor where you already have :hover. So if you click it you leave :focus on it whereas :focus-visible only applies to forms of focus which should display a visible highlight for a11y
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.
I removed most of that code 7f7cd1b
keyboard navigation is still working.
Hope it fixes the problem? I really don't know much about css
t3chguy marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
Check warning on line 203 in apps/web/src/components/views/dialogs/devtools/StickyEventState.tsx
SonarQubeCloud / SonarCloud Code Analysis
Use <input type="button">, <input type="image">, <input type="reset">, <input type="submit">, or <button> instead of the "button" role to ensure accessibility across all devices.
See more on https://sonarcloud.io/project/issues?id=element-web&issues=AZzCyOF0ytpodCISuISp&open=AZzCyOF0ytpodCISuISp&pullRequest=32741
Outdated
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.
CSS please.
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.
done here eabe6b2
I kept just the width part that change for each column, I have seen it done in other parts of the code
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.
done
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.
I kept just the width part that change for each column, I have seen it done in other parts of the code
That doesn't make it right unfortunately, we have a lot of tech debt given the age of the codebase
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.
done
Check warning on line 349 in apps/web/src/components/views/dialogs/devtools/StickyEventState.tsx
SonarQubeCloud / SonarCloud Code Analysis
`new Error()` is too unspecific for a type check. Use `new TypeError()` instead.
See more on https://sonarcloud.io/project/issues?id=element-web&issues=AZzCyOF0ytpodCISuISq&open=AZzCyOF0ytpodCISuISq&pullRequest=32741
Uh oh!
There was an error while loading. Please reload this page.