Skip to content

Add some generic tools to improve reusability#2260

Merged
offtherailz merged 2 commits intogeosolutions-it:masterfrom
offtherailz:general_tools
Oct 5, 2017
Merged

Add some generic tools to improve reusability#2260
offtherailz merged 2 commits intogeosolutions-it:masterfrom
offtherailz:general_tools

Conversation

@offtherailz
Copy link
Copy Markdown
Member

@offtherailz offtherailz commented Oct 3, 2017

Description

This pull request provides some useful enhancers to speed up coding and reuse code.

  • A generic component for toolbars (with buttons configurations, glyphs and tooltip management)
  • A tooltip enhancer. To avoid to add OverlayTrigger stuff every time.
  • A generic empty page component and enhancer (that allows to simply render a configurable empty page on some conditions
  • A Sidebar tool with good defaults, to reuse.

Please check if the PR fulfills these requirements

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

  • Feature
  • Code style update (formatting, local variables)

What is the current behavior? (You can also link to an open issue here)
Nothing.

What is the new behavior?
Developers can use the new tools to implement a more coherent UI and reusing most of the code.

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

  • No

Other information:
Toolbar
code:
image
result:
image

Empty State
code:
image
result:
image

 - A generic component for toolbars (with buttons configurations, glyphs and tooltip management)
 - A tooltip enhancer. To avoid to add OverlayTrigger stuff every time.
 - A generic empty page component and enhancer (that allows to simply render a configurable empty page on some conditions
 - A Sidebar tool with good defaults, to reuse.
@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.05%) to 80.267% when pulling d6d603d on offtherailz:general_tools into 81e62d3 on geosolutions-it:master.

Copy link
Copy Markdown
Contributor

@mbarto mbarto left a comment

Choose a reason for hiding this comment

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

Looks good, just minor comments

description,
content
} = {}) =>
(<div style={{textAlign: "center", position: "absolute", width: "100%"}} >
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.

Let's use className and css

* @param {string|node} [content] Additional content for the empty view (e.g. buttons...)
*/
module.exports = ({
opacity= 0.45,
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.

css?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I will place it in css. Anyway I should keep the possibility to override the main div style (and then the stylesheet opacity) to allow quick fix of edge cases.

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.

Having a style property (with empty defaults) is ok



/**
* Empty State enhancer. Enhances an object adding a (localizable) tooltip.
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.

I think this documentation is copied from tooltip.jsx

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

good catch. Going to fix it

@ghost ghost assigned offtherailz Oct 4, 2017
@ghost ghost added the pending review label Oct 4, 2017
@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.05%) to 80.267% when pulling 462d461 on offtherailz:general_tools into 81e62d3 on geosolutions-it:master.

@offtherailz offtherailz merged commit e1e3afd into geosolutions-it:master Oct 5, 2017
@ghost ghost removed the pending review label Oct 5, 2017
@offtherailz offtherailz deleted the general_tools branch June 21, 2018 07:26
offtherailz added a commit to offtherailz/MapStore2 that referenced this pull request Jan 17, 2019
@allyoucanmap allyoucanmap mentioned this pull request Aug 9, 2024
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants