Repository metrics
- Stars
- (1,348 stars)
- PR merge metrics
- (PR metrics pending)
Description
This issue is to follow up on #4745 with the refactorings mentioned there, as described originally in chat.
First, consolidate the two props narrow and stream into just narrow:
But it seems like there's also a deeper issue, which is that this code is doing its work using
streambut it's relying onnarrowto tell it whether, basically, the data instreamis meaningful.That sets us up for this kind of bug. Instead, […] probably actually the right thing is that
MentionWarningsshould take just aNarrow. After all, that "stream" value is just derived straight fromnarrowanyway, by the caller.
Then also, that prop should be called instead like destinationNarrow:
Along with removing the
streamprop, I thinkMentionWarningscould reasonably grow an invariant that the "reply narrow" or "destination narrow"—the answer to the question, "where should replies to this message go?"—is a stream narrow or a topic narrow. Its caller could conditionally not call it, otherwise, and it could do a check, e.g., in its constructor. […]Yeah, agreed -- the narrow that
MentionWarningsis passed should be the destination narrow, not the narrow you're viewing. And then its name should reflect that explicitly.We already do that for
ComposeMenu:<ComposeMenu destinationNarrow={this.getDestinationNarrow()}so we can easily start passing the same thing to
MentionWarnings.