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

Greatly improve performance of channel settings emote list #599

Merged
merged 2 commits into from
Jul 15, 2016

Conversation

calzoneman
Copy link
Owner

The channel settings emote list is now paginated and leverages the same
basic code as the emote browser, but with a different renderer. Fixes
#594 and kills an ugly function.

The channel settings emote list is now paginated and leverages the same
basic code as the emote browser, but with a different renderer.  Fixes
 #594 and kills an ugly function.
return;
}
var end = Math.min(start + this.itemsPerPage, this.emotes.length);
var self = this;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Closing the edit dialogue jumps you back to page 1. 😡 That's gonna piss me off (and probably others) to no end. It's especially annoying since it happens even if you haven't modified it.

var self = this; this.page = page;

@Xaekai
Copy link
Collaborator

Xaekai commented Jul 12, 2016

Damn, you beat me to it. Of course your code looks much nicer than what I would have thought of initially 😥

Longstanding annoyance (affects the emotelist too): The prev << and next >> buttons of NewPaginator should be their own blocks with .float-left and .float-right respectively, so they are in a consistent position so you can spam click them to get where you want to be. Currently the width of the button block changes a lot because of the changing button sizes (especially from page 1 to page 13), and whether break buttons ... appear.. I was planning on making a PR for this myself, but thought I'd mention it.

row.appendChild(tdDelete);

// Add emote name
// TODO: editable
Copy link
Collaborator

Choose a reason for hiding this comment

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

This seems like busy work I could handle after this is merged.

@Xaekai
Copy link
Collaborator

Xaekai commented Jul 12, 2016

While you're at it, you could move showChannelSettings, startQueueSpinner, and stopQueueSpinner (lines 3017-3062) right above EmoteList, that way the emote list classes are directly adjacent to each other and it doesn't hurt the organization of the rest of the file any. Just an idea.

@calzoneman
Copy link
Owner Author

The client is going to be factored out to use webpack or something similar to break it down into smaller modules. At that time, EmoteList and CSEmoteList will become separate files in the source tree that depend one on the other.

@calzoneman
Copy link
Owner Author

Pushing to beta, the paginator button spacing issue can be the subject of a different change.

@Xaekai
Copy link
Collaborator

Xaekai commented Jul 14, 2016

Tested. Looks ready for merge.

@calzoneman calzoneman merged commit 31a392c into 3.0 Jul 15, 2016
@calzoneman calzoneman deleted the improve-cs-emotelist branch July 15, 2016 06:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants