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.

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

Only change nick autocompletion when receiving a message #1495

Merged
merged 1 commit into from Sep 5, 2017

Conversation

xPaw
Copy link
Member

@xPaw xPaw commented Sep 4, 2017

Loading more history would mess with autocompletion order (it would also sort for each message when loading the page, which is completely unnecessary).

@xPaw xPaw added the Type: Bug Issues that report and PRs that solve any defects that cause unexpected behaviors. label Sep 4, 2017
@xPaw xPaw added this to the 2.5.0 milestone Sep 4, 2017
@@ -44,7 +44,7 @@ function appendMessage(container, chanId, chanType, msg) {

// Insert date marker if date changed compared to previous message
if (prevMsgTime.toDateString() !== msgTime.toDateString()) {
lastChild = $(templates.date_marker({msgDate: msg.time}));
lastChild = $(templates.date_marker({time: msg.time}));
Copy link
Member Author

Choose a reason for hiding this comment

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

Missed this in previous PR.

time: msg.time,
previews: []
});
const newCondensed = $(templates.msg_condensed({time: msg.time}));
Copy link
Member Author

Choose a reason for hiding this comment

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

There's no need to build a complete message since it uses a separate template so we can just optimize it.

@@ -77,12 +73,6 @@ function appendMessage(container, chanId, chanType, msg) {

function buildChatMessage(chanId, msg) {
Copy link
Member

Choose a reason for hiding this comment

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

ChanId should be removed... which will create a fun diff. Feel free to leave it out of this PR for that reason.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's called in a single place, so it's easy.

And other minor optimizations and fixes
@xPaw xPaw force-pushed the xpaw/only-order-autocomplete-on-msg branch from 8fa373d to e2a122c Compare September 5, 2017 15:28
@astorije astorije merged commit 87d6918 into master Sep 5, 2017
@astorije astorije deleted the xpaw/only-order-autocomplete-on-msg branch September 5, 2017 23:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Issues that report and PRs that solve any defects that cause unexpected behaviors.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants