-
-
Notifications
You must be signed in to change notification settings - Fork 278
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
Don't load a titlePane/floating/mobile sidebar widget until the parent widget is opened. #730
Conversation
Should this be the default or optional?
I think optional. If the user doesn't say wait, it should load in the background. I think that's better in general anyways, what would be an example of a widget you wouldn't want to load immediately? I'm also looking at it from the perspective of using the build system in which all of the widgets will be loaded and available immediately. |
An example of I am looking at this from a performance perspective and improving the user experience. Specifically for large applications with many (too many?) layers and widgets, the initial UI gets blocked waiting for everything to load. The wait is often for network calls to services. A good example is a large application that reuses a single map service multiple times instead of building many services (side note: this saves precious server resources like memory so leads to significant savings in hardware and licensing dollars). Layer control makes multiple requests to the server to get the legend. Legend widget does the same thing. That is the most basic example. A number of other widgets do many similar requests and it all adds up. WAB widgets are quite slow to load even with no network requests. Waiting on all these requests at start up can be significant. Loading a widget on demand when it is used for the first time, helps tremendously speeding up the app load time. Obviously, it has to be done in a a smart way since some widgets depend on other widgets and just assume they will be available waiting for a topic to be published. Related to the example I mentioned for LayerControl, I am working on a caching system for requests for legend info so only one request is made per service. I have done this for another large network-intensive widget that displays legend swatches. A build system helps some with this because the modules are pre-loaded but does nothing for network requests related to data. Those requests are my primary target here. Loading widget modules later in a non-build application is a nice side benefit. Another free benefit is for mobile applications or when network is slow. The app starts quicker - build system or not. Future enhancements can be to cache those network requests in local storage but we haven't gotten there yet and that caching can be complicated (but not impossible) for "live" data. Overall, I see very few issues with the approach and immediate performance benefits. I've done basic testing with core widgets, contributed widgets, a few WAB widgets and some very complex custom widgets including "heavy" applications with a large mix of widgets and layers. Based on this, I suggest it be the default. Any titlePane widget that is open at startup or has Thoughts? |
I see the benefit now. So the widgets wouldn't be started until the user needs them. That could definitely benefit built or non built applications, I agree. The layer control wouldn't make its requests until its opened, etc. I wonder though, should we |
The delay to load the widget modules when opened is typically pretty minimal (WAB seems to be an exception). Also the current process will work for more widgets since not all of them have a startup method or wait for startup to do heavy lifting. The discussion of whether they should have a useful startup method is a topic for another discussion... 🤔 |
Since I don't believe in coincidences, I'll just say that the timing of this newly submitted issue is |
I notice there is an issue with this approach and the mobile sidebar. Investigating... |
Upon investigation, this issue with WAB widgets is unrelated is unrelated. All the WAB widgets I tested do seem to work fine with this lazy-loading of widgets. I was not quite correct regarding the issue I mentioned with mobile sidebar. There isn't an error. However, the widgets in the mobile sidebar currently do not take advantage of this lazy-loading approach. Consideration for a future PR.... |
Lazy loading widgets reduces application "ready" time dramatically. Users love it. What I like to do is have a widget property called |
@DavidSpriggs thanks for that explanation, I will definitely be giving that a shot |
It gets a bit more complicated putting widget widget css in the layer file though, not sure how to deal with that. Would all the individual widgets use the same |
Add this to the build profile and any css that is loaded via xstyle will be included into the built JS file for the widget: plugins: {
"xstyle/css": "xstyle/build/amd-css"
} (see : https://github.com/kriszyp/xstyle/blob/master/build/sample-dojo-profile.js) All other core css should be combined into one css file using import statements. During the build all imports are brought into that one css file. |
Yep, I have already configured that. But the css gets dumped into the core css file. https://github.com/cmv/cmv-app-dojo-builder/blob/master/profiles/build.profile.js#L231 |
Unless you don't specify |
Not sure if I'm just doing this wrong, or if the build system won't work like this...but if I separate say the LayerControl into its own layer file, it requires lots of its own dependencies that are already bundled in the other layers. Like |
@DavidSpriggs I used a property |
@roemhildtg It should exclude already included files. I would have to look at the build profile. Send me a link. |
this allows support for lazy-load of floating widgets. also allows re-usability for other widgets.. add support for iconClass for Floating widgets.
…ePane.css rearrange some styles in cmv-theme-overrides.css improved button css in cmv-theme-overrides.css for flat theme.
…bar. switch configuration option from `loadOnOpen: false/true` to `preload: true/false`.
@DavidSpriggs @roemhildtg I've expanded support for lazy-loading the widget to include floating widgets and when the widget is loaded in the mobile sidebar. How's it look? |
viewer/css/cmv-theme-overrides.css
Outdated
} | ||
|
||
.flat .dijitTitlePaneTitle { | ||
border: 1px solid #DDD; | ||
border-bottom: none; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe it should be
.flat .dijitTitlePaneTitle.dijitOpen {
border-bottom: none;
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oops. removed.
Okay, awesome. I like this a lot, it makes it more tempting to switch back to the main cmv from the dojo-builder! I think this is ready to merge then, correct? |
Yes. I consider it merge-ready. 😄 |
Should this be the default or optional? For example, the Identify widget should probably not be delayed...