This repository has been archived by the owner on Sep 11, 2024. It is now read-only.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Poll history: detail screen #10172
Poll history: detail screen #10172
Changes from 5 commits
2c791ea
ca130f0
5456698
2a168a9
ac2b24e
0983cea
61854d8
58d5ef2
9688076
a673eeb
1c9af42
d6b0e6d
95cca0a
b88c5f9
21360db
b8fa1b8
822f0b8
506a0eb
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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 looks to me like it ought to introduce a type-checking error, and I'm slightly mystified as to why it doesn't. Am I missing some reason why we can rely on
permalinkCreator
to be defined despite the?
inIProps
?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 feels like the right solution here is to make
permalinkCreator
a required prop forRoomSummaryCard
and add a!
in RightPanel. YMMV though)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.
createDialog
has very soft typesThere 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.
Ah that explains why typescript isn't catching it. Still feel like it's a thing that we should address though.
To expand on my thinking here: AIUI currently we're assuming that whenever RightPanel is given a Room, it is also given a permalink creator. That's okayyyy though I wish that assumption was made explicit either via comments or, better, via the typing system. Still it's not a new situation so doesn't particularly need fixing in this PR. What we can do in this PR is limit the impact of that assumption by sticking a
!
in RightPanel rather than softening the types for RoomSummaryCard.