feat: Devtool for sticky events MSC4354#32741
Conversation
apps/web/src/components/views/dialogs/devtools/StickyEventState.tsx
Outdated
Show resolved
Hide resolved
f955d14 to
adcba32
Compare
|
@AndrewFerr FYI If I have some spare time later, I will see how to add a delayed event devtool. Will think about how to mix them for sticky delayed |
apps/web/src/components/views/dialogs/devtools/StickyEventState.tsx
Outdated
Show resolved
Hide resolved
apps/web/src/components/views/dialogs/devtools/StickyEventState.tsx
Outdated
Show resolved
Hide resolved
apps/web/src/components/views/dialogs/devtools/StickyEventState.tsx
Outdated
Show resolved
Hide resolved
apps/web/src/components/views/dialogs/devtools/StickyEventState.tsx
Outdated
Show resolved
Hide resolved
|
|
||
| const rowStyle: React.CSSProperties = { | ||
| cursor: "pointer", | ||
| background: hover ? "rgba(0,0,0,0.03)" : "transparent", |
There was a problem hiding this comment.
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.
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.
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.
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
apps/web/src/components/views/dialogs/devtools/StickyEventState.tsx
Outdated
Show resolved
Hide resolved
| <th style={{ textAlign: "left", padding: "8px 12px", width: "35%" }}>{_t("devtools|users")}</th> | ||
| <th style={{ textAlign: "left", padding: "8px 12px", width: "50%" }}> | ||
| {_t("devtools|sticky_key")} | ||
| </th> | ||
| <th style={{ textAlign: "right", padding: "8px 12px", width: "15%" }}> | ||
| {_t("devtools|expires_in")} | ||
| </th> |
There was a problem hiding this comment.
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.
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
|
Added a quick commit 0d458b7 to display when the homeserver does not support sticky events, as I noticed that matrix.org does not.
|
| const [eventType, setEventType] = useState<string | null>(null); | ||
| const [event, setEvent] = useState<MatrixEvent | null>(null); |
There was a problem hiding this comment.
The preferred way is this
| 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
| const cli = useContext(MatrixClientContext); | ||
| // Check if the server supports sticky events and show a message if it doesn't. | ||
| // undefined means we are still checking, true/false means we have the result. | ||
| const [stickyEventsSupported, setStickyEventsSupported] = useState<boolean | undefined>(undefined); |
There was a problem hiding this comment.
| const [stickyEventsSupported, setStickyEventsSupported] = useState<boolean | undefined>(undefined); | |
| const [stickyEventsSupported, setStickyEventsSupported] = useState<boolean>(); |
| useEffect(() => { | ||
| cli.doesServerSupportUnstableFeature("org.matrix.msc4354") | ||
| .then((result) => { | ||
| setStickyEventsSupported(result); | ||
| }) | ||
| .catch((ex) => { | ||
| logger.warn("Failed to check if sticky events are supported", ex); | ||
| }); | ||
| }, [cli]); |
| const refresh = (): void => setEvents([...context.room._unstable_getStickyEvents()]); | ||
| context.room.on(RoomStickyEventsEvent.Update, refresh); | ||
|
|
||
| return () => { | ||
| context.room.off(RoomStickyEventsEvent.Update, refresh); | ||
| }; |
There was a problem hiding this comment.
this should use useTypedEventEmitterState
dbkr
left a comment
There was a problem hiding this comment.
Generally looking good though I think
| return <p>{_t("devtools|no_sticky_events")}</p>; | ||
| } | ||
|
|
||
| const onAction = async (): Promise<void> => { |
There was a problem hiding this comment.
Why's this async? Also probably ought to be useCallback().
There was a problem hiding this comment.
It is because BaseTool IProps expects onAction(this: void): Promise<string | void>
| */ | ||
| const StickyEventTableLine: React.FC<StateEventButtonProps> = ({ userId, stickyKey, expiresAt, onClick }) => { | ||
| const [timeRemaining, setTimeRemaining] = useState<string>(""); | ||
| const [isExpired, setIsExpired] = useState<boolean>(false); |
There was a problem hiding this comment.
This state feels a bit redundant? Isn't it just a function of timeRemaining?
|
Thanks! |

Adds a new devtool the explore the current sticky state of a room. A bit similar to the room state explorer.
Checklist
public/exportedsymbols have accurate TSDoc documentation.