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

feature: new group options #1910

Merged
merged 7 commits into from
Dec 4, 2023
Merged

feature: new group options #1910

merged 7 commits into from
Dec 4, 2023

Conversation

jokd
Copy link
Contributor

@jokd jokd commented Nov 28, 2023

Fixes #1896
Adds three new options for groups:

"description": "Group descripton", // If set, this is shown on second page along with opacityControl (if set)
"opacityControl": true, // If set, this is shown on second page along with description (if set). Changing the opacity will change the opacity for all layers in the group. It will also affect groups in the group.
"zoomToExtent": true, // If set to true and there is an extent set for the group a link in the popupMenu will be created

Example config:

{
      "name": "grupp",
      "title": "grupp",
      "expanded": true,
      "groups": [
        {
          "name": "grupp2",
          "title": "grupp2",
          "expanded": true,
          "abstract": "Kort abstract",
          "description": "Detta är en grupp för test, visas på sida 2",
          "opacityControl": true,
          "zoomToExtent": true,
          "extent": [
            -20026376.39,
            -20048966.10,
            20026376.39,
            20048966.10
          ]
        },{
          "name": "grupp3",
          "title": "grupp3"
        }
      ]
    }

@johnnyblasta
Copy link
Collaborator

Why only for subgroups?
Isn't there a case where you'd want this on a main group?

Copy link
Collaborator

@johnnyblasta johnnyblasta left a comment

Choose a reason for hiding this comment

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

Tested and it works fine with the test cases I throw at it.

@jokd
Copy link
Contributor Author

jokd commented Nov 29, 2023

Why only for subgroups? Isn't there a case where you'd want this on a main group?

I thought subgroups are more similar to layers and could have this functionality but perhaps you are right and it should be possible. However I think groups are handled a bit different than subgroups but can have a look at it.

@johnnyblasta
Copy link
Collaborator

Why only for subgroups? Isn't there a case where you'd want this on a main group?

I thought subgroups are more similar to layers and could have this functionality but perhaps you are right and it should be possible. However I think groups are handled a bit different than subgroups but can have a look at it.

Yes, the handling is different and therefore I approved it after testing, but the user might not understand why it wouldn't be possible to set on main group also.

@jokd jokd changed the title feature: new subgroup options feature: new group options Nov 30, 2023
@jokd
Copy link
Contributor Author

jokd commented Nov 30, 2023

Why only for subgroups? Isn't there a case where you'd want this on a main group?

I thought subgroups are more similar to layers and could have this functionality but perhaps you are right and it should be possible. However I think groups are handled a bit different than subgroups but can have a look at it.

Yes, the handling is different and therefore I approved it after testing, but the user might not understand why it wouldn't be possible to set on main group also.

I had another go at it and now it will work on top level groups as well.
It will also fix #1640

Added description for layer and option to show abstract for groups on properties page as well. This way layer and groups behave quite similar.
@filleg
Copy link
Contributor

filleg commented Nov 30, 2023

Tried it with the latest changes and it works fine on the main group, but I can't get the moreinfo button to work on the subgroups.
Nice feature btw! Seems to cover #1492 as well.

@jokd
Copy link
Contributor Author

jokd commented Nov 30, 2023

Tried it with the latest changes and it works fine on the main group, but I can't get the moreinfo button to work on the subgroups. Nice feature btw! Seems to cover #1492 as well.

Had totally forgotten about that issue and it looks spot on this.
Whats your issue with subgroups?

@jokd
Copy link
Contributor Author

jokd commented Nov 30, 2023

Another thought; Perhaps the option to showAbstract shouldnt be about whether to show abstract on the properties-page as well as the legend but rather on properties page instead of the legend?
I bet @asemoller has opinons on this

@filleg
Copy link
Contributor

filleg commented Nov 30, 2023

Tried it with the latest changes and it works fine on the main group, but I can't get the moreinfo button to work on the subgroups. Nice feature btw! Seems to cover #1492 as well.

Had totally forgotten about that issue and it looks spot on this. Whats your issue with subgroups?

The moreinfo button works when only setting opacityControl:true, but nothing happens when I click if I also add zoomToExtent. Works well on the main group however.

@jokd
Copy link
Contributor Author

jokd commented Nov 30, 2023

Tried it with the latest changes and it works fine on the main group, but I can't get the moreinfo button to work on the subgroups. Nice feature btw! Seems to cover #1492 as well.

Had totally forgotten about that issue and it looks spot on this. Whats your issue with subgroups?

The moreinfo button works when only setting opacityControl:true, but nothing happens when I click if I also add zoomToExtent. Works well on the main group however.

Right on; might have messed up the popup when adding the group functionality. Will fix.

@asemoller
Copy link
Contributor

You bet I do!

Abstract, description and showAbstract work fine. My only input is that the abstract/description is a bit confusing. Abstract's main position should be in the group info page just like it is for layers and not in the legend as it is now. If you want additional information that appears directly in the legend, it should be called description. So switching these two should be more user friendly for admins. But I understand if you don't want to make that change because backwards compatibility will be a bit damaged but I think it will be better in the end. As for showAbstract, I'm not convinced. Personally, I don't think that the same information should be shown in two places. But that's just me!

Really nice improvements!

@jokd
Copy link
Contributor Author

jokd commented Nov 30, 2023

OK new suggestion:
abstract and description are both to be found on the properties page, unless showAbstractInLegend is set to true and then it will behave like before. I suggest we default this to false so the abstract will move to the properties page unless configured to do so.

The reason for having both abstract and description as options is that some plugins/functions use abstract for adding layer information, the desciption option is a way to add another string.

@jokd
Copy link
Contributor Author

jokd commented Nov 30, 2023

Tried it with the latest changes and it works fine on the main group, but I can't get the moreinfo button to work on the subgroups. Nice feature btw! Seems to cover #1492 as well.

Had totally forgotten about that issue and it looks spot on this. Whats your issue with subgroups?

The moreinfo button works when only setting opacityControl:true, but nothing happens when I click if I also add zoomToExtent. Works well on the main group however.

Fixed now, managed to trigger click event twice

@johnnyblasta
Copy link
Collaborator

Tested again and still approve.
Good work!

@jokd jokd merged commit 39a43aa into master Dec 4, 2023
4 checks passed
@jokd jokd deleted the new-group-options branch December 4, 2023 13:10
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.

New group options
4 participants