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.

Sign up for GitHub

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

Jump to bottom

Fix #3608 improved tooltip customization in TOC #3618

Merged
merged 2 commits into from
Mar 14, 2019
Merged

Fix #3608 improved tooltip customization in TOC #3618

merged 2 commits into from
Mar 14, 2019

Conversation

MV88
Copy link
Contributor

@MV88 MV88 commented Mar 13, 2019 •
edited
Loading

Description

This pr enhances the way tooltips for TOC nodes are configured and customized

As described in TOC plugin documentation you can:

  • disable the tooltip input fields by adding
{
"name": "TOC",
  "cfg": {
    "layersOptions": {
      "showTooltipOption": false
    }
  }
}

or customize some options like max length of the tooltip or the join string

{
"name": "TOC",
  "cfg": {
    "layersOptions": {
      "truncateLength": 200,
      "joinStr": " : "
    }
  }
}

Issues

Please check if the PR fulfills these requirements

What kind of change does this PR introduce? (check one with "x", remove the others)

  • Bugfix (there were a bug when updating description in a group/subgroup)
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Other... Please describe:

What is the current behavior? (You can also link to an open issue here)
you cannot setup tooltip options for each node and subgroup

What is the new behavior?
you can change tooltip options for each node (groups, subgroups, and layers)

Does this PR introduce a breaking change? (check one with "x", remove the other)

  • Yes
  • No

If this PR contains a breaking change, please describe the impact and migration path for existing applications: ...

Other information:
also added some English translation to non supported ones

this is a screenshot of the new UI
image

image

@MV88 MV88 self-assigned this Mar 13, 2019
@MV88 MV88 requested a review from offtherailz March 13, 2019 11:52
@coveralls
Copy link

coveralls commented Mar 13, 2019 •
edited
Loading

Coverage Status

Coverage increased (+0.006%) to 80.94% when pulling 748d338 on MV88:3608_improvements into cd62de6 on geosolutions-it:master.

offtherailz
offtherailz requested changes Mar 13, 2019
View reviewed changes
Copy link
Member

@offtherailz offtherailz left a comment •
edited by MV88
Loading

Choose a reason for hiding this comment

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

  • Can be the 2 tooltip select put inline?
  • I see a lot of changes to the translation files. There was something missing? If so, please specify this in the PR

web/client/components/TOC/DefaultGroup.jsx Outdated
@@ -29,7 +29,8 @@ class DefaultGroup extends React.Component {
selectedNodes: PropTypes.array,
onSelect: PropTypes.func,
titleTooltip: PropTypes.bool,
tooltipOptions: PropTypes.object
truncateLength: PropTypes.number,
Copy link
Member

Choose a reason for hiding this comment

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

use a tooltipOptions object to wrap all the things and change the names (joinString --> separator) (truncateLength --> maxLength)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tdipisa should we align the two tooltips?

@offtherailz regarding the translations, i just added some missing translations to the non mantained ones

web/client/selectors/__tests__/layers-test.js
"nodeType": "groups",
"options": {}
};
const flat = [
Copy link
Member

Choose a reason for hiding this comment

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

Please remove all the layers not necessary for this test

web/client/translations/data.nl-NL Outdated
@@ -45,7 +50,7 @@
"transparent": "Transparant",
"singleTile": "Single Tile",
"cached": "Gebruik cache-opties",
"styleCustom": "Gebruik een stijl genaamd \"{value} \"",
"styleCustom": "G ebruik een stijl genaamd \"{value} \"",
Copy link
Member

Choose a reason for hiding this comment

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

I think there is a typo here

Suggested change
"styleCustom": "G ebruik een stijl genaamd \"{value} \"",
"styleCustom": "Gebruik een stijl genaamd \"{value} \"",

web/client/utils/LayersUtils.js Outdated
* adds or update node property in a nested node
* similar to deepchange but it overrides a whole group of options instead of one
*/
const deepOptionsChange = (nodes, findId, propValues = {}) => {
Copy link
Member

@offtherailz offtherailz Mar 13, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

I think this can be implemented by iterating on propValues keys for deepChange (or vice-versa deepChange can be implemented using this). There is a little of redundancy.
It's deepChange enough tested to replace the implementation? otherwise you can skip, maybe comment

@MV88
improve and clean up code, fix, style, tests and translations
748d338 
* also added tocitem settings in hte dcoma config
@geosolutions-it geosolutions-it deleted a comment Mar 13, 2019
@tdipisa tdipisa merged commit 9aa0e0a into geosolutions-it:master Mar 14, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@offtherailz offtherailz offtherailz approved these changes

Assignees

@MV88 MV88

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

TOC mouse over tooltip customization
4 participants
@MV88 @coveralls @offtherailz @tdipisa