Skip to content

Fix #2983 remove + button when loading an existing dashboard (#2983)#3011

Merged
offtherailz merged 4 commits intogeosolutions-it:masterfrom
baloola:reomve_plus_sign
Jun 21, 2018
Merged

Fix #2983 remove + button when loading an existing dashboard (#2983)#3011
offtherailz merged 4 commits intogeosolutions-it:masterfrom
baloola:reomve_plus_sign

Conversation

@baloola
Copy link
Copy Markdown
Contributor

@baloola baloola commented Jun 19, 2018

Description

This PR provide a fix for the issue below

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
  • 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)
#2983

What is the new behavior?
dashboard editor with the connect button, when the page is loading. In the case of new dashboard entry point it appears normally

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:
the test was conducted in chromium

@ghost ghost assigned baloola Jun 19, 2018
@tdipisa tdipisa requested a review from offtherailz June 19, 2018 13:10
@coveralls
Copy link
Copy Markdown

coveralls commented Jun 19, 2018

Coverage Status

Coverage decreased (-24.5%) to 56.056% when pulling 984d4bd on baloola:reomve_plus_sign into bdd58b9 on geosolutions-it:master.

@MV88 MV88 added this to the 2018.03.00 milestone Jun 19, 2018
hasConnections: groups.length > 0,
hasWidgets,
canEdit: (resource ? resource.canEdit : true),
canEdit: (resource ? resource.canEdit : (isNaN(path.substr(-4))) ? true : false),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Seems a good solution.
Can you isolate canEdit in a selector for dashboard and write a unit test?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ok

getWidgetsDependenciesGroups,
(showConnections, logged, resource, hasWidgets, groups = []) => ({
pathnameSelector,
(showConnections, logged, resource, hasWidgets, groups, path = []) => ({
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

here groups are losing the default value, is it intended?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

not intended

Copy link
Copy Markdown
Contributor Author

@baloola baloola Jun 20, 2018

Choose a reason for hiding this comment

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

i couldn't detect a change in groups value.
Could you elaborate please

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

with this pr you are removing = [] from groups property.
you should restore it like this
(showConnections, logged, resource, hasWidgets, groups = [], path = [])

@MV88
Copy link
Copy Markdown
Contributor

MV88 commented Jun 20, 2018

Just a minor thing.
In the description of the PRs, check the tests checkbox only if you have added a(some) unit test(s)
Same policy for docs
;)

@tdipisa tdipisa modified the milestones: 2018.03.00, 2018.02.00 Jun 20, 2018
@offtherailz offtherailz changed the title remove + button when loading an existing dashboard (#2983) Fix #2983 remove + button when loading an existing dashboard (#2983) Jun 21, 2018
@offtherailz offtherailz merged commit 3cd29c0 into geosolutions-it:master Jun 21, 2018
offtherailz pushed a commit to offtherailz/MapStore2 that referenced this pull request Jul 17, 2018
@baloola baloola deleted the reomve_plus_sign branch August 9, 2018 10:03
@chiaracurcio chiaracurcio self-requested a review August 9, 2018 13:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remove + appearing before the dashboard is loaded, in view mode

7 participants