Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings
This repository was archived by the owner on Sep 11, 2024. It is now read-only.

Conversation

florianduros
Copy link
Contributor

@florianduros florianduros commented Aug 22, 2024

Checklist

  • Tests written for new code (and old code if feasible).
  • New or updated public/exported symbols have accurate TSDoc documentation.
  • Linter and other CI checks pass.
  • Sign-off given on the changes (see CONTRIBUTING.md).

Part of https://github.com/element-hq/element-internal/issues/609
Figma https://www.figma.com/design/nwbnk8RaZctjrrRGTUcx1X/Pinned-Messages?node-id=24-126291&t=21A0TwvtjtfbAidQ-0

  • Extract the pinned event hooks of the pinned message list to a dedicated hook files
  • Add a banner at the top of a room to display the pinned messages.

@florianduros florianduros added the T-Enhancement New features, changes in functionality, performance boosts, user-facing improvements label Aug 22, 2024
@florianduros florianduros force-pushed the florianduros/pinned-messages/banner branch from a6bb865 to f341ee2 Compare August 22, 2024 13:33
@florianduros florianduros force-pushed the florianduros/pinned-messages/banner branch from f341ee2 to 23b39af Compare August 22, 2024 16:46
@florianduros florianduros force-pushed the florianduros/pinned-messages/banner branch from 5c26f96 to 0e3c560 Compare August 26, 2024 08:29
@florianduros florianduros force-pushed the florianduros/pinned-messages/banner branch from 0e3c560 to 1f9dde9 Compare August 26, 2024 09:03
@florianduros florianduros force-pushed the florianduros/pinned-messages/banner branch from 1f9dde9 to b5895b4 Compare August 26, 2024 09:33
@florianduros florianduros force-pushed the florianduros/pinned-messages/banner branch from b5895b4 to 6ce4cc8 Compare August 27, 2024 08:43
@florianduros florianduros force-pushed the florianduros/pinned-messages/banner branch from a8f359e to 809790d Compare August 29, 2024 07:52
src/components/views/rooms/PinnedMessageBanner.tsx Outdated Show resolved Hide resolved
src/components/views/rooms/PinnedMessageBanner.tsx Outdated Show resolved Hide resolved
}

// Handle poll events
await room.processPollEvents([event]);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, really feels like this is pulling logic out of the js-sdk and duplicating it here. Is there any way we could let the js-sdk do all this work?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I copy pasta this logic from the previous impl of pinning. You are talking about the manual event fetching right?

Copy link
Contributor Author

@florianduros florianduros Aug 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dbkr My proposition is leave this duplication here since the duplication existed before this PR. And I'll make a dedicated PR to move all the ( https://github.com/matrix-org/matrix-react-sdk/blob/develop/src/components/views/messages/MPollEndBody.tsx#L41 ) duplicates into the js-sdk.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, sounds ideal if you have the time - thank you!

test/components/views/rooms/PinnedMessageBanner-test.tsx Outdated Show resolved Hide resolved
@florianduros florianduros added this pull request to the merge queue Aug 29, 2024
Merged via the queue into develop with commit d16ab09 Aug 29, 2024
@florianduros florianduros deleted the florianduros/pinned-messages/banner branch August 29, 2024 14:42
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

T-Enhancement New features, changes in functionality, performance boosts, user-facing improvements

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.