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

Introduce "reload" js controller method #3774

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

AntonKhorev
Copy link
Collaborator

Part of 2d18621.

This is a method that gets called

  • for controllers that have it defined
  • when the "Reload current page" button is pressed in the browser
  • if it's possible to detect that this button is pressed

Otherwise the usual "load" method is called.

If any kind of client-side caching is used, js controllers can flush the cache in this method.

@@ -179,8 +187,20 @@ OSM.Router = function (map, rts) {
};

router.load = function () {
var loadState = currentRoute.run("load", currentPath);
var loadState = loadOrReload();
Copy link
Member

Choose a reason for hiding this comment

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

What's the reason for putting this logic in a nested function if this is the only user?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Are you asking why it's nested or why it's defined when there's only one caller?

The short answer is to initialize var loadState when it's declared.

Copy link
Member

Choose a reason for hiding this comment

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

I'm asking why put the logic in a function rather than just having it inline.

Are you saying that function is only called once rather than each time the load function is invoked?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Of course loadOrReload is called as many times as router.load.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Changed it to extra variable instead of extra function.

if (currentRoute.has("reload") && window.performance) {
var wasReloaded = window.performance.getEntriesByType("navigation").some(function (entry) {
return entry.type === "reload";
});
Copy link
Member

Choose a reason for hiding this comment

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

I'm not familiar with window.performance but from a quick bit of reading on MDN it seems this if testing if there are any navigation events of type reload in the history but what's the scope of that? The comment you added says we're interest in whether this request was a reload?

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 think it's for the current page only, not for the history.

https://developer.mozilla.org/en-US/docs/Web/API/Performance : "The Performance interface provides access to performance-related information for the current page".

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