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

Show subscription on reddit #1031

Merged
merged 6 commits into from
May 13, 2014

Conversation

matheod
Copy link
Collaborator

@matheod matheod commented May 11, 2014

See #1027.

var url = '';
var threads = $("#newCommentsTable tr"); // use the table to keep the selected sorting
for(var i = 1, len = threads.length; i < len; i++) { // i = 1 to avoid the table header
url += ',t3_' + $(threads[i]).find('td:first span[thread]').attr('thread');
Copy link
Collaborator

Choose a reason for hiding this comment

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

why not use RES's data directly? this is the dashboard, RES creates all the DOM itself.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

To allow people to sort by clicking on header of the table, then opening link in the sorted way choosen.

Copy link
Collaborator

Choose a reason for hiding this comment

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

i suppose that's an okay reason.

I still have some nitpicks with that for loop, but it's just style-- not mission-critical

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yea true, now it's better.

@matheod matheod changed the title Show subscription on redit Show subscription on reddit May 11, 2014
Replace this horrible for for a pretty each
@@ -58,6 +58,16 @@ modules['newCommentCount'] = {
var showDiv = $('<div class="show">Show:</div>')
var subscriptionFilter = $('<select id="subscriptionFilter"><option>subscribed threads</option><option>all threads</option></select>')
$(showDiv).append(subscriptionFilter);
var openOnReddit = $('<div class="addButton" id="openOnReddit">Open on Reddit</div>');
$(openOnReddit).click(function() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This can simply be openOnReddit.click since openOnReddit is already a jQuery object: ideally, you should rename it to $openOnReddit so it's clear that it is a jQuery object. Same with the showDiv variable above.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I used the same notation than above in the code (I.e. showDiv for exemple).

matheod added 2 commits May 12, 2014 21:26
Thanks mc10
Thanks andytuba
honestbleeps added a commit that referenced this pull request May 13, 2014
@honestbleeps honestbleeps merged commit b72bfc7 into honestbleeps:master May 13, 2014
@jewel-andraia jewel-andraia mentioned this pull request May 22, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants