-
-
Notifications
You must be signed in to change notification settings - Fork 171
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
Refactor message2 #1147
Refactor message2 #1147
Conversation
instead of looping over the dom afterwards
also importify media (the gallery component)
fix logger on message list importify attachment
f1e74b4
to
d1f1abc
Compare
<MenuItem onClick={onReply}> | ||
{tx('reply_to_message_desktop')} | ||
</MenuItem> | ||
*/} |
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 should be in the next release
import logger from '../../../logger' | ||
|
||
import { getLogger } from '../../../logger' | ||
const log = getLogger('render/msgList') |
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 fixed the logger here.
} | ||
} | ||
} | ||
|
||
const onClickSetupMessage = setupMessage => openDialog('EnterAutocryptSetupMessage', { setupMessage }) | ||
const onShowDetail = message => openDialog('MessageDetail', { message, chat }) |
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.
why do we have to reuse the chat object here? I would appreciate it when that dialog could fetch it from the core again, that would allow us to move this function around and inside of the message.
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.
same on delete message, the dependency to the chatStore is annoying
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.
Also I would appreciate if we could call openDialog without a ScreenContext.
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.
why do we have to reuse the chat object here? I would appreciate it when that dialog could fetch it from the core again, that would allow us to move this function around and inside of the message.
The message object should be enough for both? I think we shouldn't refetch anything that we already have, also because this could introduce inconsistencies if something changed in between. ChatStore is annoying anyways... openDialog without a screenContext sounds not so easy for me, how do you want to achieve this? Global variable?
remote.dialog.showSaveDialog({ | ||
defaultPath | ||
}, (filename) => { | ||
if (filename) ipcRenderer.send('saveFile', msg.file, filename) |
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 you use the ipcBackend variable from the rendered/ipc file? This makes it easier to switch to websockets/whatever at some point.
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.
doesn't need to be done now IMO, I just copied it over and we need to revisit this soon anyway, to remove the remote
thing
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.
👍
see commit messages for details.
In essence I moved more stuff between files in hope to make the code more readable and prepare merging of message-wrapper and message.