Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fixes [6654] Clicking on user avatar (profile picture) in header doesn't open conversation details #6658

Closed
wants to merge 23 commits into from
Closed

fixes [6654] Clicking on user avatar (profile picture) in header doesn't open conversation details #6658

Changes from 2 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
ec5705c
Strict types in RemoteConfig
automated-signal Nov 2, 2023
1bb1e3a
Call Controls: Enhance width for viewport sizes
automated-signal Nov 2, 2023
361468c
Reduce flake and extend edit-test to include more send state updates
automated-signal Nov 3, 2023
650c2f5
Username and username link integrity check
automated-signal Nov 3, 2023
29ffac9
Updated div to be inside button
neverstopgrindingbhaskar Nov 5, 2023
795293e
Fix untranslated labels/tooltips in NavTabs
automated-signal Nov 6, 2023
62ac48f
Add remote config for sending higher res screen share
automated-signal Nov 6, 2023
3f30a7b
Use Message received_at_ms to hide typing bubble
automated-signal Nov 6, 2023
d22828d
Better check for empty storage/master keys
automated-signal Nov 7, 2023
0de7f40
Fix call settings and participant page buttons
automated-signal Nov 7, 2023
6842129
Remove deprecated ContactDetails fields
automated-signal Nov 7, 2023
15ddae1
Fix infinite stacking All Media with shortcut
automated-signal Nov 7, 2023
7106352
Fix UI bug with self-add by username in groups
automated-signal Nov 8, 2023
842332a
Fix left pane banner colors
automated-signal Nov 8, 2023
308270f
Show error when editing messages after max edits
automated-signal Nov 8, 2023
8c2ce6b
avatar to call pushconversation instead of undef
neverstopgrindingbhaskar Nov 8, 2023
20356ac
Fix group call video alignment in RTL
automated-signal Nov 8, 2023
e6de7d8
Backport Language Picker
jamiebuilds-signal Nov 8, 2023
16186d5
Update WhatsNew
jamiebuilds-signal Nov 8, 2023
db78a63
Update strings
jamiebuilds-signal Nov 8, 2023
824d252
v6.39.0-beta.1
jamiebuilds-signal Nov 8, 2023
7e02619
Updated renderAvatar to accept callback
neverstopgrindingbhaskar Nov 10, 2023
70c7d2a
Merge branch 'signalapp:main' into clickingAvatar
bhaskarraksahb Nov 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 7 additions & 4 deletions ts/components/conversation/ConversationHeader.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ export class ConversationHeader extends React.Component<PropsType, StateType> {
storyViewMode: StoryViewModeType.User,
});
}
: undefined
: this.getActionForType()
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like this function would be called a duplicate time from renderHeader().
Also renderAvatar() is only called from renderHeader().

What if we change renderAvatar() to take the onClick handler generated earlier?
It can look like this to indicate it's a fallback handler in case the stories handler isn't needed:

  private renderAvatar(onClickFallback: undefined | (() => void)): ReactNode {

Then it will be simpler and we would not recreate the handler function.

}
phoneNumber={phoneNumber}
profileName={profileName}
Expand Down Expand Up @@ -679,7 +679,7 @@ export class ConversationHeader extends React.Component<PropsType, StateType> {
);
}

private renderHeader(): ReactNode {
private getActionForType() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Combined with the change to renderAvatar(), we would not need to recreate the onClick handler.
In that case it's ok to revert this change and keep onClick generation as part of renderHeader().

(As a side note, if hypothetically we wanted to keep this new onClick generator, it's maybe more clear to name it verbosely such as getHeaderOnClickHandler() because "getActionForType" can be confusing in a complex component.)

const { groupVersion, pushPanelForConversation, type } = this.props;

let onClick: undefined | (() => void);
Expand All @@ -703,7 +703,10 @@ export class ConversationHeader extends React.Component<PropsType, StateType> {
default:
throw missingCaseError(type);
}

return onClick;
}
private renderHeader(): ReactNode {
Copy link
Contributor

@ayumi-signal ayumi-signal Nov 8, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Same as above, I think we can revert this change and keep onClick generation as part of renderHeader(). Then we can pass the onClick to renderAvatar().

const onClick = this.getActionForType();
const avatar = this.renderAvatar();
const contents = (
<div className="module-ConversationHeader__header__info">
Expand Down Expand Up @@ -905,4 +908,4 @@ function OutgoingCallButtons({
default:
throw missingCaseError(outgoingCallButtonStyle);
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

The app's eslint style guide specifies trailing newlines. Can you revert this newline deletion?

Loading