Skip to content

Conversation

@ahocevar
Copy link
Member

With this change, the drag box we use for ol.interaction.DragZoom and ol.interaction.DragBox is just a DOM element with CSS styling. This way it works for all renderers.

Fixes #1583.

css/ol.css Outdated
Copy link
Member

Choose a reason for hiding this comment

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

why not simply blue?

Copy link
Member Author

Choose a reason for hiding this comment

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

Because the old style had color: [0, 0, 255, 1]. But you're right, blue would be simpler.

@bartvde
Copy link
Member

bartvde commented Oct 15, 2015

Theoretically drag zoom and drag box could have different styles in the past, but not in your new approach. Should we have 2 css classes?

@bartvde
Copy link
Member

bartvde commented Oct 15, 2015

Btw, so happy to see this PR!

@fredj
Copy link
Member

fredj commented Oct 15, 2015

Theoretically drag zoom and drag box could have different styles in the past, but not in your new approach. Should we have 2 css classes?

A className could be added to olx.interaction.DragBoxOptions

@ahocevar
Copy link
Member Author

Good point. Instead of removing the style constructor property, I could replace it with a className property, using ol-dragbox and ol-zoombox respectively. Makes sense?

@ahocevar
Copy link
Member Author

@fredj You were faster typing what we both thought at the same time it seems :-)

@bartvde
Copy link
Member

bartvde commented Oct 15, 2015

Makes perfect sense. You and @fredj had the same thought.

@fredj
Copy link
Member

fredj commented Oct 15, 2015

:-)

@fredj
Copy link
Member

fredj commented Oct 15, 2015

And the box-selection example should be updated:

diff --git a/examples/box-selection.js b/examples/box-selection.js
index ded8174..69ed2a9 100644
--- a/examples/box-selection.js
+++ b/examples/box-selection.js
@@ -8,9 +8,6 @@ goog.require('ol.layer.Tile');
 goog.require('ol.layer.Vector');
 goog.require('ol.source.OSM');
 goog.require('ol.source.Vector');
-goog.require('ol.style.Fill');
-goog.require('ol.style.Stroke');
-goog.require('ol.style.Style');


 var vectorSource = new ol.source.Vector({
@@ -44,15 +41,7 @@ var selectedFeatures = select.getFeatures();

 // a DragBox interaction used to select features by drawing boxes
 var dragBox = new ol.interaction.DragBox({
-  condition: ol.events.condition.platformModifierKeyOnly,
-  style: new ol.style.Style({
-    fill: new ol.style.Fill({
-      color: [255, 255, 255, 0.4]
-    }),
-    stroke: new ol.style.Stroke({
-      color: [100, 150, 0, 1]
-    })
-  })
+  condition: ol.events.condition.platformModifierKeyOnly
 });

 map.addInteraction(dragBox);

@ahocevar
Copy link
Member Author

Good catch. Will do @fredj.

@bartvde
Copy link
Member

bartvde commented Oct 15, 2015

Other than that this LGTM @ahocevar

@ahocevar
Copy link
Member Author

Thanks for the great suggestions. I just force-pushed a new version of the pull request.

@ahocevar
Copy link
Member Author

@fredj, do you want to take another look? In addition to what you suggested, I also gave olx.interaction.ZoomBoxOptions a className property, and added css to the box-selection example to match the previous styles.

@fredj
Copy link
Member

fredj commented Oct 15, 2015

LGTM thanks

ahocevar added a commit that referenced this pull request Oct 15, 2015
Use DOM instead of map canvas for ol.render.Box
@ahocevar ahocevar merged commit fcdff1c into openlayers:master Oct 15, 2015
@ahocevar ahocevar deleted the css-box branch October 15, 2015 16:00
@1Map
Copy link

1Map commented Jan 26, 2016

In ol 3.10 I had the following:

        ol.events.condition.ctrlKeyOnly = function(mapBrowserEvent) {
            var browserEvent = mapBrowserEvent.browserEvent;
            return (!browserEvent.shiftKey && browserEvent.ctrlKey);
        };

        this.circleInteraction = new ol.interaction.DragBox({
            disabled : true,
            condition : ol.events.condition.ctrlKeyOnly,
            style : new ol.style.Style({
                fill : new ol.style.Fill({
                    color : [255, 0, 0, 0.3]
                }),
                stroke : new ol.style.Stroke({
                    color : [255, 0, 0, 1],
                    width : 1
                })
            })
        });

How would I implement the above with the latest version?

Before it was:

this.box_ = new ol.render.Box(style);

Now it is:

this.box_ = new ol.render.Box(options.className || 'ol-dragbox');

@ahocevar
Copy link
Member Author

The v3.11 upgrade notes should more or less answer your question. Define your drag box like this:

this.circleInteraction = new ol.interaction.DragBox({
    disabled : true,
    condition : ol.events.condition.ctrlKeyOnly,
    className: 'dragbox-circle'
});

And css like this:

.dragbox-circle {
  border-color: rgba(255,0,0,1);
  border-width: 1px;
  background-color: rgba(255,0,0,0.3);
}

Let us know if anything remains unclear.

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.

4 participants