-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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.
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
Conversations panel (inbox) #4980
Conversation
</Box> | ||
) | ||
|
||
const rowBorderColor = (idx: number, lastParticipantIndex: number, hasUnread: boolean, isSelected: boolean) => { |
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.
In the multi avatar case we actually have a blue border around the second avatar. If there is an unread then we orange circle the last avatar (can only be one also)
const participants = participantFilter(conversation.get('participants')) | ||
const isSelected = selectedConversation === conversation.get('conversationIDKey') | ||
const isMuted = conversation.get('muted') | ||
// $FlowIssue |
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.
flow doesn't like me passing avatar props into the multi since its a union. dunno what a good fix is
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.
Can we do something like: https://github.com/keybase/client/blob/master/shared/chat/conversation/index.desktop.js#L16 so types will still help us?
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.
ah I think I understand, it won't even type to Array properly
marginLeft: -5, | ||
marginTop: 5, | ||
// TODO remove this when we get the updated icon w/ the stroke | ||
textShadow: ` |
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 was just to get it kinda looking close. I asked @cecileboucheron to give us a perfect icon instead of using the iconfont
@@ -58,7 +58,7 @@ class Avatar extends PureComponent<void, Props, State> { | |||
...avatarStyle, | |||
...borderStyle, | |||
display: (!showNoAvatar && !showLoadingColor) ? 'block' : 'none', | |||
backgroundColor: globalColors.white, | |||
backgroundColor: this.props.backgroundColor || globalColors.white, |
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.
if this is white always you can get bad aliasing on the edges on dark backgrounds due to the alpha blend on the border radius
@keybase/react-hackers this is ready for a look |
…-conversation-panel
@keybase/react-hackers bump |
@@ -50,6 +50,3 @@ const rightAvatar = { | |||
} | |||
|
|||
export default MultiAvatar |
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.
Affected by this eslint import issue: import-js/eslint-plugin-import#660
@keybase/react-hackers bump |
1 similar comment
@keybase/react-hackers bump |
return isSelected ? globalColors.darkBlue2 : globalColors.darkBlue4 | ||
} | ||
return hasUnread ? globalColors.orange : undefined | ||
} |
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.
🐑 🐑 I think this might be an easier to understand format:
if (idx === lastParticipantIndex && lastParticipantIndex === 1 && hasUnread) {
return globalColors.orange
}
if (idx === lastParticipantIndex && lastParticipantIndex === 1 && isSelected) {
return globalColors.darkBlue2
}
if (idx === lastParticipantIndex && lastParticipantIndex === 1) {
return globalColors.darkBlue4
}
if (idx === lastParticipantIndex && hasUnread) {
return globalColors.orange
}
return undefined
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.
i changed mine to be more readable i think, take a look in the next commit
const participants = participantFilter(conversation.get('participants')) | ||
const isSelected = selectedConversation === conversation.get('conversationIDKey') | ||
const isMuted = conversation.get('muted') | ||
// $FlowIssue |
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.
Can we do something like: https://github.com/keybase/client/blob/master/shared/chat/conversation/index.desktop.js#L16 so types will still help us?
const participants = participantFilter(conversation.get('participants')) | ||
const isSelected = selectedConversation === conversation.get('conversationIDKey') | ||
const isMuted = conversation.get('muted') | ||
// $FlowIssue |
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.
ah I think I understand, it won't even type to Array properly
github seems fucked |
Gets the inbox panel pretty close. There's a todo to fix up the 'shh' icon but lets do that in a different ticket