-
-
Notifications
You must be signed in to change notification settings - Fork 113
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
fix map staying blank if no permissions on default theme #197
base: master
Are you sure you want to change the base?
fix map staying blank if no permissions on default theme #197
Conversation
@@ -94,6 +94,20 @@ class AppInitComponent extends React.Component { | |||
if (ConfigUtils.getConfigProp("dontLoadDefaultTheme")) { | |||
return; | |||
} | |||
|
|||
// Determine default theme, and fallback if not available | |||
let availableThemes = themes.items.map((t) => t.id); |
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.
Theme items might potentially be nested.
@@ -94,6 +94,20 @@ class AppInitComponent extends React.Component { | |||
if (ConfigUtils.getConfigProp("dontLoadDefaultTheme")) { | |||
return; | |||
} | |||
|
|||
// Determine default theme, and fallback if not available | |||
let availableThemes = themes.items.map((t) => t.id); |
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.
Theme items might potentially be nested.
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.
Ok will try to come up with a commit that handles nested themes. Would you have a sample themes.json test with, or some info in how to create projects that have nested themes ? Is it with QGIS themes in the project ?
(In my themes.json, I see sublayers, but nothing looking like nested themes, not even an empty list)
I wonder whether a better approach would be to be able to set the default theme per user. This way it might be hard to control which theme is shown in the end. |
Currently (if I get things right), I agree it would be better to be able to define it per user, but where would that happen exactly ? I'm not yet too familiar with the QWC2 stack, but guess it needs to be handled by the auth backend somehow, and probably not really is attached to the user, but rather to roles. And then what happens if a user has many roles ? IMO that's quite a big change for such a small fix, and may become quite difficult to correctly setup. By using the first one, it's deterministic so one can still use alphabetic ordering to determine which one gets loaded. By the way, as said in qwc-services/qwc-map-viewer#12, qwc-map-viewer could fix that on the fly instead of fixing it on the JS side. |
This works around the issue of map staying blank if the logged user has no permission on the default map. Such a situation easily happens with the stock qwc-map-viewer and qwc-config-generator, which assumes the first project is the default project, not taking into account user permissions.
See qwc-services/qwc-map-viewer#12 for more context.