Major improvements in code extensibility, reduced API usage, eliminate redundancy, etc.

[GaurangTandon]

My main motive in this four days attempt was to first reduce the heavy API usage by introducing filters. As I got engrossed in the code further, I managed to fix issues related to code extensibility, redudancy, refactoring, caching, etc. So, this turned out to be a slightly larger update than expected.

This PR builds on the code directly from the latest branch (dev2.2.15). I have tested all the functions that I changed. They all work, except for a few quirks that were present since before.

These are the most important changes:

1. remove synchronous ajax call - "Synchronous XMLHttpRequest outside of workers is in the process of being removed from the web platform as it has detrimental effects to the end user’s experience." under getFromAPI
2. throttled all observer calls - it is a given that MutationObserver fires dozens of times a second, but the user we're designing this app for won't even notice changes faster than 250ms.
3. the apiParameter list in common.info.json was redundant, since none of the files are actually using the corresponding methods apiParameter or metaApiParameter. So, I removed that.
4. parseCrossSiteLinks now also works for bare links in comments
5. four new helper functions under sox.helpers - as they are frequently used in the code
6. redefined the constant currentAPIParameter using the helper method above

Apart from that, all the other changes fix bugs, or make the code extensible, faster, less redundant, etc. However, there are a few things I could not fix:

1. downvotedPostsEditAlert - I don't know where to begin refactoring this from. It has three API calls, and more than four hundred lines of code. I can't even comprehend how it is doing what it's supposed to do. Perhaps, you might want to refactor it later yourself. At least reduce its length, its huge :O
2. What is the $anchor in showQuestionStateInSuggestedEditReviewQueue? I think you forgot to define that variable.
3. What is the need for Yahoo! YQL API in standOutDupeCloseMigrated? The SE API already provides us with migrated_from and migrated_to details.
4. How do you handle more than one dupe links to be clicked on the clickable stamp for standOutDupeCloseMigrated? It is a known fact that moderators can mark more than one dupe targets for a single question.
5. hotNetworkQuestionsFiltering is unable to hide those links which are under the "more hot questions" link. As soon as you click that link, all the list items under that automatically appear. I think it needs an onclick handler for that.
6. standOutDupeCloseMigrated dupe stamp link is /q/ID when it is set, but becomes /undefined when you click on it.
7. the getQuestionTags part of hotNetworkQuestionsFiltering has been buggy since previous version. there is no next element sibling of i.getQuestionTags

---

Since this is a big change, feel free to go through it manually and comment and ping me for changes on individual lines - I'll be more than happy to explain my changes. Even though I have tested all the changed methods, feel free to test them again :)

And when you're happy, we can push these changes, possibly under a new 2.3.0 version number ;)

[shu8]

@GaurangTandon Wow, thanks a lot for your time! I will go through this as soon as I can, but I'm sure it'll be fine!

I'll also have a look at the points you mentioned and try fixing them too :) I was thinking of moving downvotedPostsEditAlert back out into a separate userscript too, because it's too big to maintain in SOX!

[shu8]

Were changes to this function by accident? (It seems to have rolled back my changes)

[GaurangTandon]

Probably. I do recall starting work on the latest dev branch. Ah...you went from dev*.*.14 to dev*.*.15 by the time I finished work, Hadn't noticed that. Sorry! Feel free to roll back to your original.

[shu8]

Is there any point in using vanilla JS just to make it a jQuery object straight after? :/

[GaurangTandon]

Actually, $(document.getElementById(ID)) is quite faster than $("#ID")...

[shu8]

@GaurangTandon fair enough, I never knew that!

[shu8]

@GaurangTandon merged! thanks again for all the changes -- it must have taken quite some time! :)

[GaurangTandon]

That's wonderful :D

[shu8]

@GaurangTandon I've fixed most of the issues you mentioned, but I couldn't reproduce the duplicate button's link being broken -- does it still happen?

I have no idea why I used YQL for the migrated questions 😆 I also had something in there to check for 'migrated from' but AFAIK the migrated notice only shows on the 'from' site?

For multiple duplicates, it just shows one of them...

I'll be taking out downvotedPostsEditAlert soon.

[GaurangTandon]

I couldn't reproduce the duplicate button's link being broken -- does it still happen?

If I go to this search page, and click the first duplicate link, it immediately transforms from /q/46898 into /undefined, taking me to stackoverflow.com/undefined :/ The link also changes if you right click on this link (verify in the dev tools).

I've little clue as to why this happens. If it possible that you have a rogue click event handler (probably a delegated one) somewhere in the code? :O

I'm testing on a fresh Chrome profile specifically for testing SOX only, so effect of other addons is zero.

[shu8]

@GaurangTandon very weird :/ it only happens on the search page, not on any other page e.g. https://stackoverflow.com/questions.

I'll look at features that act on the search page...

[shu8]

Fixed it!

---

If you're interested, it was:

 $(".js-search-results").on("mousedown touchstart", "div.result-link a", function (ev) {
     this.href = $(this).data("searchsession");
 });

on line 1332 of the /search page:

I honestly have no clue why it's there :/

I tried changing it to:

var question = data.items[0],
      questionId = question.closed_details.original_questions[0].question_id,
      $linkToAdd = $('<a/>', {
          'style': 'display: inline',
          'href': 'https://' + sox.site.url + '/q/' + questionId,
          'click': function(e) {
               e.preventDefault();
               return;
           }
      }),
      $spanToAdd = $('<span>', {
          'class': 'standOutDupeCloseMigrated-duplicate',
          'title': 'click to visit duplicate',
          'html': '&nbsp;duplicate&nbsp;'
     });
     
     //styling for https://github.com/soscripted/sox/issues/181
     $anchor.after($linkToAdd.append($spanToAdd));

but that click handler never got triggered, no idea why.

So, in the end, I just added a data-searchsession attribute to the sox span itself, so the search code can work for us rather than against!

[GaurangTandon]

If you're interested, it was:

Woah, how did you manage to find that line of code in the middle of nowhere o.O 10/10 great!

I honestly have no clue why it's there :/

Oh, I get it! This code must be for their internal analytics: tracking the referrer. The original .result link a had a data-searchsession attribute that reads: /questions/ID/SLUG?s=1|86.0153. The s=1|86.0153 is a query parameter that must be there to track user's clicking patterns.

but that click handler never got triggered, no idea why

I guess this is because e.preventDefault() prevents the default behavior associated with an input (for example, focusing the URL bar (default behavior) on pressing Ctrl-L (input)). The code you showed on line 1332 of the /search page is not a default behavior, so it didn't get any suppressed :(

So, in the end, I just added a data-searchsession attribute to the sox span itself, so the search code can work for us rather than against!

That is the perfect trick my friend ;) applauses

[GaurangTandon]

I've fixed most of the issues you mentioned

This is really minor so I'll comment it here. getQuestionTags works but it immediately redirects to the linked question on click, because the i.getQuestionTags itself is part of the link.

[shu8]

I'm away for a few days but I'll look at that when I get back :) I still don't get why the click handler itself never ran :/ (I added a `console.log('test');` to the `click` but it just refused to detect it! Thanks 😆

[shu8]

@GaurangTandon I'm looking at it now -- do you mean this box shouldn't be clickable?

[GaurangTandon]

Aye, this box itself, and also the i.getQuestionTags icon that is there before this box appears. Both should not be a part of the link.

[shu8]

@GaurangTandon should be fixed in dev 2.2.22 :)

[GaurangTandon]

Dude....that's probably an overkill :( Those i.getQuestionTags everywhere are getting into my eyes:

Look, the thing is that this i.getQuestionTags function isn't going to be used too often. So, we can simply set it to show the question's tags on hovering over the question link itself. Let me share a PR.

[shu8]

Fair enough, although I feel we need some visual indicator that you can hover over them, and add this to the feature's extended_desc. A simple 'hover to show tags' just under the 'Hot Network Questions' could do?

[GaurangTandon]

You will hover over the links to click them anyway. Once you hover over them, the tags list immediately pops out below the question link. Try it out! ;)

[shu8]

The idea is to see the question tags before you click.

If you don't know to hover to show the tags, there's no reason you should hover (by clicking) at all because you just wouldn't go to the question.

[GaurangTandon]

So, you're saying that once this update goes alive, any user - interested in this feature - won't hover over any HNQ question, even once, even by mistake, before thinking that showQuestionTags no longer works?

That's a bit of an overstatement in my humble opinion, considering that there is almost a negligible delay in the fetching of the tags, I have pushed another update to #351, that should make the app's response time zero. Please have a look.

[shu8]

I'm saying it's not an obvious feature, and there's no need for a user to accidentally realise they can show the tags by hovering when we can just add a line of text that makes it obvious!

[GaurangTandon]

Well, alright with that.

[shu8]

I (finally!) got round to pushing this to master under 2.3.0! Thanks a lot for your help in this version @GaurangTandon! :) 🎉

[GaurangTandon]

You're welcome, I'm glad to help :)

…

On Sun 2 Sep, 2018, 19:49 ᔕᖺᘎᕊ, ***@***.***> wrote: I (finally!) got round to pushing this to master under 2.3.0! Thanks a lot for your help in this version @GaurangTandon <https://github.com/GaurangTandon>! :) 🎉 — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#348 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AGBDSy516fAnZ0_6sVPC4mQ-TbRancymks5uW-kOgaJpZM4VI8QN> .