Close #3025 Add boundary for draggable dialog by kasongoyo · Pull Request #4025 · geosolutions-it/MapStore2

kasongoyo · 2019-07-30T11:20:52Z

Description

Limit the dialog dragging zone to be within the view port. It also includes changes to the followings dialogs;

DownloadDialog
GroupDialog
UserDialog
MeasureDialog
ShareDialog
Print
SearchServicesConfig
Settings
About

Issues

Please check if the PR fulfills these requirements

The commit message follows our guidelines: https://github.com/geosolutions-it/MapStore2/blob/master/CONTRIBUTING.md
Tests for the changes have been added (for bug fixes / features)
Docs have been added / updated (for bug fixes / features)

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

What is the current behavior? (You can also link to an open issue here)
see #3025

What is the new behavior?
The draggable dialog are by default limited to browser view port

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

Yes, and I documented them in migration notes
No

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

Other information:

coveralls · 2019-07-30T11:49:16Z

Coverage increased (+0.03%) to 82.151% when pulling 954e5a3 on kasongoyo:feature/3025 into 96a80aa on geosolutions-it:master.

offtherailz

The solution provided is too complicated, it uses the dom and a lot of hacks. (Also the dialog component is too complicated).

I suggest a simpler solution :

Add bounds as property to draggable
Set Your dialogs (I did it with About) bounds to string to bound the draggable (I used #container because body didn't allow me to move vertically. @mbarto do you suggest anything better as default container of the application to bound dialogs?)
remove, by overriding default className, the modal-dialog, that is the bootsrap one, because it adds a margin top of 150px and you don't want it.
add start: {x: 0, y: 150} to emulate replicate the initial offset.
This is more or less my changes :

           diff --git a/web/client/components/misc/Dialog.jsx b/web/client/components/misc/Dialog.jsx
index 7ef917539..72c1fb5c6 100644
--- a/web/client/components/misc/Dialog.jsx
+++ b/web/client/components/misc/Dialog.jsx
@@ -84,7 +84,7 @@ class Dialog extends React.Component {
                 {this.renderRole('footer')}
             </div> : <span/>}
         </div>);
-        const dialog = this.props.draggable ? (<Draggable defaultPosition={this.props.start} handle=".draggable-header, .draggable-header *">
+        const dialog = this.props.draggable ? (<Draggable bounds={this.props.bounds} defaultPosition={this.props.start} handle=".draggable-header, .draggable-header *">
             {body}
         </Draggable>) : body;
         let containerStyle = assign({}, this.props.style.display ? {display: this.props.style.display} : {}, this.props.backgroundStyle);
diff --git a/web/client/product/components/viewer/about/About.jsx b/web/client/product/components/viewer/about/About.jsx
index 63b99a7a1..0a2867e0f 100644
--- a/web/client/product/components/viewer/about/About.jsx
+++ b/web/client/product/components/viewer/about/About.jsx
@@ -49,7 +49,7 @@ class About extends React.Component {
             className="map-logo"
             body={
                 <AboutContent/>
-            }/>) : (<Dialog id="mapstore-about" style={assign({}, {zIndex: 1992, display: this.props.enabled ? "block" : "none"})}>
+            } />) : (<Dialog id="mapstore-about" className="modal-content" bounds="#container" start={{x: 0, y: 150}} style={assign({}, {zIndex: 1992, display: this.props.enabled ? "block" : "none"})}>
                 <span role="header"><span className="about-panel-title"><I18N.Message msgId="about_title"/></span><button onClick={this.props.onClose} className="about-panel-close close">{this.props.modalConfig.closeGlyph ? <Glyphicon glyph={this.props.modalConfig.closeGlyph}/> : <span>×</span>}</button></span>
                 <div role="body"><AboutContent/></div>
             </Dialog>);

There are around 10 usages of this dialog in MapStore. You can continue fixing using this method, for the ones you know, keeping unchanged the other ones, that you can not test.

I think this approch will fix most of the dialogs and add this functionality. An alternative solution is a refactor of the dialog default props, but this can cause some issue where this component is used.

offtherailz · 2019-07-30T16:16:09Z

web/client/components/misc/Dialog.jsx

 }

-module.exports = Dialog;
+const enhance = compose(


This is too complicated and hardly maintainable.

kasongoyo · 2019-07-31T08:52:00Z

@offtherailz But with your proposed soln, the draggable will never move off the view port. I thought user will need to drag the dialog off the view port for some percentage in order to see clearly what's behind the dialog? That's what I was hardly trying to achieve in the first place otherwise the proposed soln is simple and less complicated and I can quickly implement it

offtherailz · 2019-07-31T09:21:14Z

I see your point. Maybe windows like print or other big ones may need this. @mbarto what do you think?
By my side, I don't think is necessary to make the dialog exit, even partially, and it takes a lot in terms of maintainability and stability. I should use build-in library functionalities as much as possible.
Let's wait a comment from @mbarto

kasongoyo · 2019-07-31T09:29:30Z

@offtherailz If dialog partial exit is not a priority, I can even use bounds=parent as the default for dialog bounds and it will by default work in almost all places where dialog is used without any change since almost everywhere dialog is parented by fixed position container.

offtherailz · 2019-07-31T09:35:49Z

@kasongoyo I tried
And so most of the dialogs need the modal-dialog class to be removed, because otherwise they are not draggable up the 150px margin give by that class (e.g. About) but removing this class will move the About on the top line, that's why I suggested to add start: {x: 0,y: 150}.

Then I tried another couple of dialogs and I see that download dialog (available from the feature grid) need to be limited in width or will have a bug. And share window had problems too.

So I suggested to fix them one by one.
Please take into account the other dialogs.

tdipisa · 2019-07-31T09:52:55Z

@kaso please review the size of the issue according to the @offtherailz suggestion so I can evaluate if we can proceed with that instead of fixing only the About dialog.

kasongoyo · 2019-07-31T10:06:47Z

The dialog is currently used in 18 places. I can implement what @offtherailz suggested with some modification as the default i.e removing margin-top from modal-dialog and set start to {x:0, y:150} but I suggest bounds to start as parent instead of #selector. I think this implementation will work on many dialogs(I will verify) and only few will need some changes. So @tdipisa I estimate it to take minimum of one day task and maximum of two days.

mbarto · 2019-07-31T10:35:34Z

My opinion is that bounds=parent should work (if a dialog needs to be put out of screen to see things, the user can close it directly, and open it again later)

tdipisa · 2019-07-31T16:06:42Z

mbarto · 2019-07-31T16:09:44Z

@tdipisa why the SharePanel needs to be draggable? I agree with the other ones

offtherailz · 2019-07-31T16:15:20Z

I tried SnapShotPanel and I noticed it's not working. This is the effect.

It should be draggable as well as the print, but it have to be fixed too. Agree with @mbarto about share panel.

With not draggable, you mean even "modal" (that means dark background) ?

kasongoyo · 2019-08-01T13:13:54Z

@tdipisa I will add modal to all non draggable dialog just to emphasize that user need to interact with the dialog at that moment.

tdipisa · 2019-08-01T14:23:00Z

@mbarto @offtherailz

@tdipisa why the SharePanel needs to be draggable? I agree with the other ones

Because there is a functionality that updates the bbox URL parameter in the shared URL when you pan the map.

@offtherailz

It should be draggable as well as the print, but it have to be fixed too. Agree with @mbarto about share panel.

We have the shapshot tool working well in the c040 project (recently deployed in production) so I'm wondering why it should not work in the MS product as well, anyway check it better and open a new issue for this please. Thank you.

offtherailz

Bugs

Measure panel can be dragged only vertically
Snapshot window
As well as I try to drag the window, it moves in a fixed place and do not move anymore.
Use this configuration in desktop pluigins:

{
        "name": "Snapshot",
        "cfg": {
          "wrap": true
        }
      },

Important note

This changes may be dangerous for geonode integration (in geonode there is the print window in a JS API custom version embedded in the preview page). @tdipisa, It should be tested. What should we do in this case?

offtherailz · 2019-08-02T09:27:02Z

Notes for PM and other developers

Looking at the code I noticed these things: Please @tdipisa and @kasongoyo take a look at them.

I took a look at the code and I've seen that Dialog has 3 specialization (in misc directory)

ResizableDialog: --> wrong name, it should be named ResponsiveDialog.
ConfirmDialog --> Used In many places. I tested them here and there and they seems to work
StandardDialog --> It has been developed to be a simplification of the Dialog to allow common use cases. It's used for Map Import

I think we should in the future we should migrate everything StandardDialog (good, simple and documented interface, Dialog's one is dark and full of workarounds 😄 ), renaming it as "Dialog"
and the old Dialog should be renamed as "BaseDialog" and discouraged for direct usage.

ConfirmDialog is ok. Resizable should be removed in favor of standard.

@mbarto what do you think?

kasongoyo · 2019-08-02T09:35:23Z

@offtherailz Concern Bugs, did you test it against my last commit?

I am going to send a fix for measure dialog but according to my last commit, the snapshot dialog is not draggable and it has the modal so it shouldn't behave like how you described in the bug section. Can you confirm that?

tdipisa · 2019-08-02T12:31:37Z

@offtherailz discuss these topics in the weekly DEV meeting and open a dedicated issue if needed, then keep me updated on the status so I can schedule the work properly.

offtherailz · 2019-08-02T12:47:02Z

Ok, @kasongoyo Probably I reviewed it this morning, probably I pulled it just before your last commit.

I talked with Tobia, we should make the snapshot draggable, instead.

offtherailz

Please remove localConfig.json changes.

* 3523 manage exponential number coord editor (#3907) * Fixed exponential problem with input type number for coordiante entry * 'e' and 'E' chars are disabled for coordinate entry * both on aeronautical or decimal * updated doc * restored onkeyDown prop, and added more tests to check that is called * removed comment * Fix #3908 Time sync support for FeatureGrid (#3909) * Fix for #3845. Restored Identify and other plugins doc (#3921) * FIx #3785 Implemented Layer Filter (#3898) * Closes #3785 #3778 #3779 #3788 * Fixed lint error and added layerfilter tests * Fixes on Lorenzo's review * Added comments and fixed query tooltip string * Fixesx after Lorenzo's second review * Fixess on Lorenzo's last review * Removed console.log * Fixed btntooltip * use protocol-relative URL for CartoDB provider (#3938) * Fix 3945 zoom to feature for point now zooms correctly (#3946) * Fix 3945 zoom to feature for point now zooms correctly * fix max zoom default * added a todo for max zoom customization * fix #3915 wrong overflow in feature info settings (#3941) * Fix Glich of query panels roi styles (#3948) * Fix Glith of query panels roi styles * remove onmount double call in feature editor * Fix 3942 with correct positioning of tutorial for GFI step (#3943) * Fix 3942 with correct positioning of tutorial for GFI step * changed implementation * Fix #3928 widget builder color selector box shows wrong labels (#3937) * Fix dashboard autoreload (#3951) * Fix thematic maps color labels (#3952) * Fixes #3955 filter layer minor issues (#3956) * Fix fr-FR translation for options menu (#3961) * Fix #3913 GFI window does not close when opening the catalog (#3913) (#3944) * Fix #3913 GFI window does not close when opening the catalog (#3913) fix #3913 * move the epic to their more appropriate file * Fix #3910 Text annotations - wrong text align in preview (#3954) fix #3910 * Fix link on quickstart page (#3968) * Add infoFormat to Identify plugin documentation (#3966) * Add infoFormat to Identify plugin documentation * changed documentation message * Fix 3960 Template editor of feature info settings has slow response on typing (#3965) * add internal state to feature info editor to reduce on change calls * add default state in feature info editor component * Added missing Croatian translations & fix typos (#3959) * Added missing Croatian translations & fix typos * Fix typo * Added missing strings that were not added automatically by devs * Fix #3957 Avoid showing the MapStore version number (2) in translations (#3971) fix #3957 * #3950: the whole group containing the annotations layer disappears in 3d mode, also if it contains other layers (#3974) * #3863: improved Print plugin documentation with info on printing usin… (#3975) * #3863: improved Print plugin documentation with info on printing using custom scales * #3863: fixed typo * Fix #3940 Integrate the tooltip for metadata with missing params (#3973) * Fix #3940 Integrate the tooltip for metadata with missing params fix #3940 * sort the templateMetadata in translation * Fix #3906 the 'back' button does not undo the annotation creation (#3967) * Fix #3906 the 'back' button does not undo the annotation creation fix #3906 * improve test coverage * Fix 3976 Search Plugin improved (#3977) * Fix 3976 Search Plugin improved * sort is correct * maxRsults is configurable and limits the results size * add test to check results sorting * Update web/client/epics/search.js Co-Authored-By: Lorenzo Natali <offtherailz@gmail.com> * Update web/client/epics/__tests__/search-test.js Co-Authored-By: Lorenzo Natali <offtherailz@gmail.com> * #3972: fixed print of vector layers for solid dash stroke (#3978) * update style of annotation text symbolizer to avoid horizontal scrollbar (#3979) * #3962: removed old examples (#3986) * fix #3969 annotation description field is not clickable in safari (#3982) * #3784 Activate Sync Map by default (#3984) * Fix #3958 hide layer related buttons when removing a layer from the map (#3980) * Fix WidgetLegend preview (#3994) * Fix WidgetLegend preview * Update web/client/components/widgets/enhancers/legendWidget.js * Ewsterrenburg vietnamese translations (#3999) * Add Vietnamese translation * Removed erroneous comma * Fixing tests * removed console.log * Update featuregrid.js * Fix 3985 add catalog autosearch (#3988) * Fix 3895 catalog autosearch * Fix reducer default, fix epic for autosearch * test was hard to provide since it is using a thunk * anyway it has been tested so i removed the test for epic * added missing default in catalog reducer * #3989: fixed the broken examples (#4003) * Fix #3934 Cross layer filter: some wfs requests fails (#3991) * Fix 3817 map info url (#4000) * Wip 3817 mapinfo on map loading * Wip GetFeatureInfo after GetFeature * wip * moved queryparam logic in a separate epic * changed main comp to fetch new epic * added whitelist * add error handling for map info from url * added a generic api for queryparam actions * added possibility to restrict list of queriable layers to perform GFI * added possibility to override some request param for specific layers * * Added documentation * fix doc link * fix german translations * fix others translations * Fix documentation and make whitelist configurable * update doc * Save filterLayer in layer object (#4010) * Fix Print tool with vendor param (CQL_FILTER) (#4006) * Fix 2749 search urls for cross layer filter are compared ignoring "dirty" chars (#4004) * Fix 2749 search urls for cross layer filter are compared ignoring "dirty" url * fix test and check for dirty chars * Fix 4007 hide filter layer when no layers are present in TOC (#4008) * Fix 4007 hide filter layer whne no layers are present * even if a group is present with no layers * Fix tests * Fixed issues due to sameURL check in #4004 (#4013) * Fix #3805 User edit password field retain value across users (#4016) On editing user, the form use to retain password field across editing session. This commit fix and enable password field reset on close edit dialog * #3929 Cross layer filter cannot be disabled when filled (#3998) #3929 Cross layer filter cannot be disabled when filled * refix #2953 to work along #3929 * [docs] Update User Guide (#3995) * [docs] Update TOC section * [docs] filtering layers * [docs] Search tool updates * [docs] metadata template for CSW services * [docs] custom tooltip in layer settings * [docs] updating 'quering objects' section with coordinates editor * [docs] updating annotations section * [docs] layer filter persistence * [docs] crs selector section (#4012) * #3622: some fixes to involved documentations (#4018) * Some updates to the CRS selector documentation (#4019) * #3817 Fixing documentation (#4023) * #3817 fixing documentation * Update docs/developer-guide/map-query-parameters.md Co-Authored-By: mbarto <maurobartolomeoli@gmail.com> * Update docs/developer-guide/map-query-parameters.md Co-Authored-By: mbarto <maurobartolomeoli@gmail.com> * Update .travis.yml (#4027) * Remove IN PROGRESS (#4026) * Close #3025 Add boundary for draggable dialog (#4025) * Close #3025 Add boundary for draggable dialog * simplification of dialog boundary solution * make snapshot dialog draggable

…tions-it#4025) * Close geosolutions-it#3025 Add boundary for draggable dialog * simplification of dialog boundary solution * make snapshot dialog draggable

Portings for the following PRs from master b to branch 2019.02.xx: - #3988 - #4016 - #3998 - #4003 - #3991 - #4004 - #4006 - #4008 - #4025 - #4071 - #4019 - #4076 - #4077 - #4081 Target release is o add them to 2019.02.01

Close geosolutions-it#3025 Add boundary for draggable dialog

e4f5240

kasongoyo force-pushed the feature/3025 branch from 0e9a7dc to e4f5240 Compare July 30, 2019 11:24

tdipisa requested a review from allyoucanmap July 30, 2019 13:12

offtherailz requested changes Jul 30, 2019

View reviewed changes

tdipisa added this to the 2019.02.01 milestone Jul 31, 2019

simplification of dialog boundary solution

1b577b3

offtherailz requested changes Aug 2, 2019

View reviewed changes

kasongoyo force-pushed the feature/3025 branch from 2b98e20 to 022accc Compare August 2, 2019 10:11

giohappy mentioned this pull request Aug 2, 2019

Test new feature that adds boundary to draggable windows GeoNode/geonode-mapstore-client#15

Closed

offtherailz approved these changes Aug 2, 2019

View reviewed changes

make snapshot dialog draggable

954e5a3

kasongoyo force-pushed the feature/3025 branch from 309af00 to 954e5a3 Compare August 2, 2019 16:24

tdipisa merged commit 81fae70 into geosolutions-it:master Aug 5, 2019

kasongoyo deleted the feature/3025 branch August 5, 2019 08:44

tdipisa assigned kasongoyo Aug 9, 2019

offtherailz mentioned this pull request Aug 9, 2019

2019.02.01 Portings #4084

Merged

Conversation

kasongoyo commented Jul 30, 2019 • edited by tdipisa Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Issues

Uh oh!

coveralls commented Jul 30, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

offtherailz left a comment

Choose a reason for hiding this comment

Uh oh!

offtherailz Jul 30, 2019

Choose a reason for hiding this comment

Uh oh!

kasongoyo commented Jul 31, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

offtherailz commented Jul 31, 2019

Uh oh!

kasongoyo commented Jul 31, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

offtherailz commented Jul 31, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tdipisa commented Jul 31, 2019

Uh oh!

kasongoyo commented Jul 31, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mbarto commented Jul 31, 2019

Uh oh!

tdipisa commented Jul 31, 2019 • edited by kasongoyo Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mbarto commented Jul 31, 2019

Uh oh!

offtherailz commented Jul 31, 2019

Uh oh!

kasongoyo commented Aug 1, 2019

Uh oh!

tdipisa commented Aug 1, 2019

Uh oh!

offtherailz left a comment

Choose a reason for hiding this comment

Bugs

Important note

Uh oh!

offtherailz commented Aug 2, 2019

Notes for PM and other developers

Uh oh!

kasongoyo commented Aug 2, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tdipisa commented Aug 2, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

offtherailz commented Aug 2, 2019

Uh oh!

offtherailz left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

kasongoyo commented Jul 30, 2019 •

edited by tdipisa

Loading

coveralls commented Jul 30, 2019 •

edited

Loading

kasongoyo commented Jul 31, 2019 •

edited

Loading

kasongoyo commented Jul 31, 2019 •

edited

Loading

offtherailz commented Jul 31, 2019 •

edited

Loading

kasongoyo commented Jul 31, 2019 •

edited

Loading

tdipisa commented Jul 31, 2019 •

edited by kasongoyo

Loading

kasongoyo commented Aug 2, 2019 •

edited

Loading

tdipisa commented Aug 2, 2019 •

edited

Loading