Bookmark stars no longer show up in side panel

[nexushoratio]

Just noticed this when I synced my personal fork to head.

The above image was captured with beta IITC core and released Bookmarks (though also happens with beta Bookmarks).

I'll start culprit finding.

[nexushoratio]

Culprit:
c5c728a

[nexushoratio]

The star will show up briefly, then disappear.

Clicking the same portal a second time, it will show up.

I noticed this comment in the bookmark code:

ingress-intel-total-conversion/plugins/bookmarks.js

Line 331 in 86900f9

// the sidebar is constructed after firing the hook

I suppose that is no longer true?

Also, it appears that the portalSelected event is no longer fired on initial page load when the site it loaded with a portal selected, like in https://intel.ingress.com/?pll=37.423521,-122.089649.

Though, previously, it was fired four times during the initial page load!

[nexushoratio]

Regarding the initial page load: The call stack changed so much I can't figure out where it might be appropriate to add the new forceSelect parameter to renderPortalDetails. There is a lot of new code with a bunch of different calls paths, and it is not clear to me where a new argument should be plumbed in.

[le-jeu]

renderPortalDetails is likely called twice when you select a portal :

1. The available data is displayed
   • if data is not fresh, request portal datails
2. On portal details response to update display

The select event now occurs once at step 1

following this piece of comment :

  // only run the hooks when we have a portalDetails object - most plugins rely on the extended data
  // TODO? another hook to call always, for any plugins that can work with less data?
  if (hasFullDetails) {
    window.runHooks('portalDetailsUpdated', { guid: guid, portal: portal, portalDetails: details, portalData: data });
  }

it may be useful to introduce a dedicated hook for portalDetailsDisplayed event (currently, Bookmarks use a timeout to hack its way)

[nexushoratio]

it may be useful to introduce a dedicated hook for portalDetailsDisplayed event (currently, Bookmarks use a timeout to hack its way)

It seems like portalDetailsUpdated is already documented to fill that niche. Is another hook needed, or documentation updated?

ingress-intel-total-conversion/core/code/hooks.js

Line 34 in 86900f9

* - `portalDetailsUpdated`: Fired after the details in the sidebar have been (re-)rendered.

While it seems like bookmarks has likely been abusing this particular hook, it might seem like this is Hryum's Law being invoked. I wouldn't be surprised that, when this was originally written, portalSelected was the only option available.

Hmm, I thought there was comment in the wiki somewhere about being careful about compatibility, but I can't find it quickly.

[nexushoratio]

The following seems sufficient. It works well because the handler only deals with global variables. Whether a good idea or not, I'll leave up to you all. :->

diff --git a/plugins/bookmarks.js b/plugins/bookmarks.js
index 625f98ea..39b6bf8b 100644
--- a/plugins/bookmarks.js
+++ b/plugins/bookmarks.js
@@ -1438,6 +1438,7 @@ var setup = function () {
   }
 
   window.addHook('portalSelected', window.plugin.bookmarks.onPortalSelected);
+  window.addHook('portalDetailsUpdated', window.plugin.bookmarks.onPortalSelected);
   window.addHook('search', window.plugin.bookmarks.onSearch);
 
   // Sync