Skip to content

Commit

Permalink
Stop hardcoding the pseudoclasses, cleanup
Browse files Browse the repository at this point in the history
For recap, these classes are how we adjust the style of features now that we
can't use CSS classes to do it.  They are strings that are owned by each layer.
We created a bunch of them like 'drawing', 'highlight', 'selected', etc.
and then at render time we call `syncFeatureClasses` to update each feature with
what kind of classes it should have.

Working on the streetlevel imagery I realized that I want more pseudo classes
to highlight image viewcones, and the list is getting long, so it's time to stop
hardcoding these special properties and handle them dynamically.

This also gives us an oppotrunity to clean up the ones that were confusing,
redundand, or not really used.

So here's what has changed:
- Removed all the AbstractFeature getter/setters and now have `setClass/unsetClass/hasClass`
  - AbstractFeature now has a private `_classes` Set<classIDs>
  - setClass/unsetClass now take care of dirtying the style and label
  - `feature.highlighted = true` -> `feature.setClass('highlight)`
  - `feature.drawing = false` -> `feature.unsetClass('drawing')`
  - etc..
- Perfer shorter names for the classes, e.g. "select" instead of "selected"
- Some renames and adjustment of arglists in the PixiScene and AbstractLayer
  - `classData(dataID, classID)` -> `setClass(classID, dataID)`
  - `unclassData(dataID, classID)` -> `unsetClass(classID, dataID)
  - etc.. the class name comes first, like `setClass('hover', 'w123')`
- Updated lots of documentation
- 'active' was used for interactivity, but it didn't really work, it's removed now
  - this only really affects `DragBehavior`, when dragging things
  - We also had `feature.allowInteraction` it does the same thing, we just use that now

This paves the way for me to do the thing I want to do with the photo viewcones
without adding more hardcoded classes.
  • Loading branch information
bhousel committed Aug 28, 2024
1 parent cd0145a commit a6ebdf9
Show file tree
Hide file tree
Showing 19 changed files with 278 additions and 333 deletions.
13 changes: 4 additions & 9 deletions modules/behaviors/DragBehavior.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,7 @@ export class DragBehavior extends AbstractBehavior {
const target = this.dragTarget;

if (eventData && target) {
target.layer.unclassData(target.dataID, 'active');
target.feature.active = false;
target.feature.allowInteraction = true;
this.dragTarget = null;
this.emit('cancel', eventData);
}
Expand Down Expand Up @@ -174,8 +173,7 @@ export class DragBehavior extends AbstractBehavior {
// This lets us catch events for what other objects it passes over as the user drags it.
const target = Object.assign({}, down.target); // shallow copy
this.dragTarget = target;
target.layer.classData(target.dataID, 'active');
target.feature.active = true;
target.feature.allowInteraction = false;

// What are we dragging?
const data = target.data;
Expand Down Expand Up @@ -244,14 +242,12 @@ export class DragBehavior extends AbstractBehavior {
}
}


this.lastDown = null;
this.lastMove = null;

const target = this.dragTarget;
if (target) {
target.layer.unclassData(target.dataID, 'active');
target.feature.active = false;
target.feature.allowInteraction = true;
this.dragTarget = null;
this.emit('end', up);
}
Expand All @@ -273,8 +269,7 @@ export class DragBehavior extends AbstractBehavior {

const target = this.dragTarget;
if (target) {
target.layer.unclassData(target.dataID, 'active');
target.feature.active = false;
target.feature.allowInteraction = true;
this.dragTarget = null;
this.emit('cancel', cancel);
}
Expand Down
6 changes: 3 additions & 3 deletions modules/core/PhotoSystem.js
Original file line number Diff line number Diff line change
Expand Up @@ -313,7 +313,7 @@ export class PhotoSystem extends AbstractSystem {
// Clear out any existing display classes
for (const oldLayerID of this._LAYERIDS) {
const oldLayer = scene.layers.get(oldLayerID);
oldLayer?.clearClass('selected');
oldLayer?.clearClass('select');
oldLayer?.clearClass('selectphoto');
}

Expand All @@ -324,8 +324,8 @@ export class PhotoSystem extends AbstractSystem {
// If we're selecting a photo then make sure its layer is enabled too.
scene.enableLayers(layerID);

scene.classData(layerID, photoID, 'selected');
scene.classData(layerID, photoID, 'selectphoto');
scene.setClass('select', layerID, photoID);
scene.setClass('selectphoto', layerID, photoID);

// Try to show the viewer with the image selected..
service.startAsync()
Expand Down
14 changes: 8 additions & 6 deletions modules/modes/DragNodeMode.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ export class DragNodeMode extends AbstractMode {
const editor = context.systems.editor;
const filters = context.systems.filters;
const l10n = context.systems.l10n;
const map = context.systems.map;
const ui = context.systems.ui;

this._reselectIDs = options.reselectIDs ?? [];
Expand Down Expand Up @@ -99,12 +100,12 @@ export class DragNodeMode extends AbstractMode {

this.dragNode = entity;
this._startLoc = entity.loc;
this._selectedData.set(entity.id, entity);

// Set the 'drawing' class so that the dragNode and any parent ways won't emit events
const scene = context.scene();
scene.classData('osm', this.dragNode.id, 'drawing');
const layer = map.scene.layers.get('osm');
layer.setClass('drawing', this.dragNode.id);
for (const parent of graph.parentWays(this.dragNode)) {
scene.classData('osm', parent.id, 'drawing');
layer.setClass('drawing', parent.id);
}

// `_clickLoc` is used later to calculate a drag offset,
Expand Down Expand Up @@ -143,8 +144,9 @@ export class DragNodeMode extends AbstractMode {
this._selectedData.clear();

const context = this.context;

context.scene().clearClass('drawing');
const map = context.systems.map;
const layer = map.scene.layers.get('osm');
layer.clearClass('drawing');

context.behaviors.drag
.off('move', this._move)
Expand Down
7 changes: 0 additions & 7 deletions modules/modes/DragNoteMode.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,10 +51,6 @@ export class DragNoteMode extends AbstractMode {
this._startLoc = note.loc;
this._selectedData.set(this.dragNote.id, this.dragNote);

// Set the 'drawing' class so that the dragNote won't emit events
const scene = context.scene();
scene.classData('notes', this.dragNote.id, 'drawing');

// `_clickLoc` is used later to calculate a drag offset,
// to correct for where "on the pin" the user grabbed the target.
const point = context.behaviors.drag.lastDown.coord.map;
Expand Down Expand Up @@ -88,9 +84,6 @@ export class DragNoteMode extends AbstractMode {
this._selectedData.clear();

const context = this.context;

context.scene().clearClass('drawing');

context.behaviors.drag
.off('move', this._move)
.off('end', this._end)
Expand Down
18 changes: 10 additions & 8 deletions modules/modes/DrawAreaMode.js
Original file line number Diff line number Diff line change
Expand Up @@ -129,9 +129,10 @@ export class DrawAreaMode extends AbstractMode {

const context = this.context;
const editor = context.systems.editor;
const scene = context.systems.map.scene;
const map = context.systems.map;
const layer = map.scene.layers.get('osm');
const eventManager = map.renderer.events;

const eventManager = context.systems.map.renderer.events;
eventManager.setCursor('grab');

context.behaviors.hover
Expand Down Expand Up @@ -177,7 +178,8 @@ export class DrawAreaMode extends AbstractMode {
this._lastPoint = null;

this._selectedData.clear();
scene.clearClass('drawing');

layer.clearClass('drawing');

window.setTimeout(() => {
context.behaviors.mapInteraction.doubleClickEnabled = true;
Expand All @@ -196,9 +198,10 @@ export class DrawAreaMode extends AbstractMode {
_refreshEntities() {
const context = this.context;
const editor = context.systems.editor;
const scene = context.systems.map.scene;
const map = context.systems.map;
const layer = map.scene.layers.get('osm');

scene.clearClass('drawing');
layer.clearClass('drawing');
this._selectedData.clear();

const graph = editor.staging.graph;
Expand All @@ -209,20 +212,19 @@ export class DrawAreaMode extends AbstractMode {

// Sanity check - Bail out if any of these are missing.
if (!drawWay || !lastNode || !firstNode) {
// debugger;
this._cancel();
return;
}

// `drawNode` may or may not exist, it will be recreated after the user moves the pointer.
if (drawNode) {
scene.classData('osm', drawNode.id, 'drawing');
layer.setClass('drawing', drawNode.id);

// Nudging at the edge of the map is allowed after the drawNode exists.
context.behaviors.mapNudge.allow();
}

scene.classData('osm', drawWay.id, 'drawing');
layer.setClass('drawing', drawWay.id);
this._selectedData.set(drawWay.id, drawWay);
}

Expand Down
18 changes: 10 additions & 8 deletions modules/modes/DrawLineMode.js
Original file line number Diff line number Diff line change
Expand Up @@ -165,9 +165,10 @@ export class DrawLineMode extends AbstractMode {

const context = this.context;
const editor = context.systems.editor;
const scene = context.systems.map.scene;
const map = context.systems.map;
const layer = map.scene.layers.get('osm');
const eventManager = map.renderer.events;

const eventManager = context.systems.map.renderer.events;
eventManager.setCursor('grab');

context.behaviors.hover
Expand Down Expand Up @@ -214,7 +215,8 @@ export class DrawLineMode extends AbstractMode {
this._lastPoint = null;

this._selectedData.clear();
scene.clearClass('drawing');

layer.clearClass('drawing');

window.setTimeout(() => {
context.behaviors.mapInteraction.doubleClickEnabled = true;
Expand All @@ -233,9 +235,10 @@ export class DrawLineMode extends AbstractMode {
_refreshEntities() {
const context = this.context;
const editor = context.systems.editor;
const scene = context.scene();
const map = context.systems.map;
const layer = map.scene.layers.get('osm');

scene.clearClass('drawing');
layer.clearClass('drawing');
this._selectedData.clear();

const graph = editor.staging.graph;
Expand All @@ -246,21 +249,20 @@ export class DrawLineMode extends AbstractMode {

// Sanity check - Bail out if any of these are missing.
if (!drawWay || !lastNode || !firstNode) {
// debugger;
this._cancel();
return;
}

// `drawNode` may or may not exist, it will be recreated after the user moves the pointer.
if (drawNode) {
scene.classData('osm', drawNode.id, 'drawing');
layer.setClass('drawing', drawNode.id);

// Nudging at the edge of the map is allowed after the drawNode exists.
context.behaviors.mapNudge.allow();
}

// todo - we do want to allow connecting a line to itself in some situations
scene.classData('osm', drawWay.id, 'drawing');
layer.setClass('drawing', drawWay.id);
this._selectedData.set(drawWay.id, drawWay);
}

Expand Down
Loading

0 comments on commit a6ebdf9

Please sign in to comment.