Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hide/show child nodes of a branch #325

Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
8f8e65a
add: hidden field to mmpnode
sorenjohanson Jul 5, 2024
bdfcfc5
add: functionality to show/hide nodes
sorenjohanson Jul 5, 2024
c867398
add: tooltip and translations
sorenjohanson Jul 5, 2024
12ff66d
changed: use hasHiddenChildren instead
sorenjohanson Jul 10, 2024
7f5f35b
Merge branch 'main' into 105-hide-show-child-nodes-of-a-branch
sorenjohanson Jul 18, 2024
204583d
changed: use client-side hidden variable, add key function to dom nodes
sorenjohanson Jul 19, 2024
068e26a
fix: do not send websocket update when hiding nodes
sorenjohanson Jul 24, 2024
32d1e1e
fix: only hide direct children, not all descendants
sorenjohanson Jul 25, 2024
b8dfccf
changed: toolbar icon depending on node visibility, removed unnecessa…
sorenjohanson Jul 25, 2024
fc9fe67
changed: remove unnecessary UserNodeProperties attribute
sorenjohanson Jul 25, 2024
68141bc
fix: hide all descendants properly based on their parent status
sorenjohanson Jul 26, 2024
84a6c25
add: hidden eye icon when updating nodes
sorenjohanson Jul 26, 2024
ea2dd7d
add: disable hide node button if no node/root node is selected
sorenjohanson Jul 26, 2024
5d4e097
changed: rename methods
sorenjohanson Jul 29, 2024
ea706ff
removed: unnecessary class
sorenjohanson Jul 29, 2024
4f197de
fix: remove text like visibility_off and link from exported image
sorenjohanson Jul 30, 2024
9ccde3b
Merge branch 'main' into 105-hide-show-child-nodes-of-a-branch
sorenjohanson Aug 9, 2024
36b5319
add: comments to explain slightly more complicated branch visibility …
sorenjohanson Aug 9, 2024
4f2288c
fix: lint
sorenjohanson Aug 9, 2024
0d010d5
changed: history should not include visibility changes, fixed: toolba…
sorenjohanson Aug 9, 2024
04e5176
changed: visibility hidden instead of filtering out nodes, set proper…
sorenjohanson Aug 10, 2024
bef9113
fix: eye icon not rendering during undo/redo
sorenjohanson Aug 12, 2024
250a84c
added: method to recursively hide child nodes and retain proper hidde…
sorenjohanson Aug 13, 2024
587016f
fix: lint
sorenjohanson Aug 13, 2024
5beaf21
changed: small changes to variable names
sorenjohanson Aug 13, 2024
d0be27c
fix: link icon disappearing
sorenjohanson Aug 13, 2024
85454a7
fix: refactoring
sorenjohanson Aug 14, 2024
ee483a7
fix: refactor updateNodeHidden
sorenjohanson Aug 14, 2024
63f58ec
changed: add comments
sorenjohanson Aug 14, 2024
de316ce
Merge branch 'main' into 105-hide-show-child-nodes-of-a-branch
sorenjohanson Aug 14, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
115 changes: 86 additions & 29 deletions teammapper-frontend/mmp/src/map/handlers/draw.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import DOMPurify from 'dompurify'
import Map from '../map'
import Utils from '../../utils/utils'
import Node from '../models/node'
import {Path} from 'd3-path'
import { Path } from 'd3-path'

/**
* Draw the map and update it.
Expand Down Expand Up @@ -55,16 +55,26 @@ export default class Draw {
* Update the dom of the map with the (new) nodes.
*/
public update() {
const nodes = this.map.nodes.getNodes(),
dom = {
nodes: this.map.dom.g.selectAll('.' + this.map.id + '_node').data(nodes),
branches: this.map.dom.g.selectAll('.' + this.map.id + '_branch').data(nodes.slice(1))
}
let nodes = this.map.nodes.getNodes()
Copy link
Member

Choose a reason for hiding this comment

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

why let here? do you need to modify this ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good catch, must've been a mistake from when I separated out nodes and dom variable creation.


// Set visibility: hidden instead of filtering out nodes to still allow updates such as text, images and pictograms to take effect "behind the curtain"
const dom = {
nodes: this.map.dom.g.selectAll('.' + this.map.id + '_node').data(nodes, (d) => d.id).style("visibility", d => d.hidden ? "hidden" : "visible"),
branches: this.map.dom.g.selectAll('.' + this.map.id + '_branch').data(nodes.slice(1), (d) => d.id).style("visibility", d => d.hidden ? "hidden" : "visible")
}
let tapedTwice = false

// When doing an initial draw, all nodes appear in dom.nodes
dom.nodes.each((node: Node) => this.checkForHiddenChildren(node))

const outer = dom.nodes.enter().append('g')
.style('cursor', 'pointer')
.style('touch-action', 'none')
/**
* dom.nodes includes all nodes rendered on screen, but dom.nodes.enter() includes "new" nodes given by the client,
* so we need an additional visibility check done here
*/
.style('visibility', (node: Node) => node.hidden ? 'hidden' : 'visible')
.attr('class', this.map.id + '_node')
.attr('id', function (node: Node) {
node.dom = this
Expand All @@ -80,7 +90,7 @@ export default class Draw {
if (!this.map.options.edit) return false
// When not clicking a link and not in edit mode, disable all mobile native touch events
// A single tap is supposed to move the node in this application
if(!this.isLinkTarget(event) && !this.editing) {
if (!this.isLinkTarget(event) && !this.editing) {
event.preventDefault()
}

Expand All @@ -97,7 +107,6 @@ export default class Draw {

this.enableNodeNameEditing(node)
})

if (this.map.options.drag === true) {
outer.call(this.map.drag.getDragBehavior())
} else {
Expand All @@ -119,16 +128,24 @@ export default class Draw {
.style('stroke-width', 3)
.attr('d', (node: Node) => this.drawNodeBackground(node))


// Set image and link of the node
outer.each((node: Node) => {
this.setImage(node)
this.setLink(node)
// Sometimes, undo/redo will not render nodes in dom.nodes, but instead all nodes will only be present in dom.nodes.enter(), so we also need to check for hidden children there
this.checkForHiddenChildren(node)
})


dom.branches.enter().insert('path', 'g')
.style('fill', (node: Node) => DOMPurify.sanitize(node.colors.branch))
.style('stroke', (node: Node) => DOMPurify.sanitize(node.colors.branch))
/**
* dom.branches includes all branches rendered on screen, but dom.branches.enter() includes "new" branches given by the client,
* so we need an additional visibility check done here
*/
.style('visibility', (node: Node) => node.hidden ? 'hidden' : 'visible')
.attr('class', this.map.id + '_branch')
.attr('id', (node: Node) => node.id + '_branch')
.attr('d', (node: Node) => this.drawBranch(node))
Expand Down Expand Up @@ -176,7 +193,7 @@ export default class Draw {
* @returns {Path} path
*/
public drawBranch(node: Node): Path {
if(node.parent === undefined || node.parent === null) return
if (node.parent === undefined || node.parent === null) return

const parent = node.parent,
path = d3.path(),
Expand Down Expand Up @@ -288,6 +305,34 @@ export default class Draw {
}
}

/**
* Set a hidden eye icon if child nodes are hidden.
* @param {Node} node
*/
public setHiddenChildrenIcon(node: Node) {
JannikStreek marked this conversation as resolved.
Show resolved Hide resolved
let domText = node.getHiddenChildIconDOM()
if (!domText) {
domText = document.createElementNS('http://www.w3.org/2000/svg', 'text')
domText.textContent = 'visibility_off'
domText.classList.add('material-icons')
domText.style.setProperty('fill', DOMPurify.sanitize(node.colors.name))
domText.setAttribute('y', (-node.dimensions.height + 30).toString())
domText.setAttribute('x', '-60')
node.dom.appendChild(domText)
}
}

/**
* Explicitly remove the hidden eye icon even if not set
* @param {Node} node
*/
public removeHiddenChildrenIcon(node) {
const domText = node.getHiddenChildIconDOM()
if (domText) {
domText.remove()
}
}

/**
* Update the node image position.
* @param {Node} node
Expand All @@ -307,7 +352,7 @@ export default class Draw {
public updateLinkPosition(node: Node) {
if (DOMPurify.sanitize(node.link.href) !== '') {
const link = node.getLinkDOM(),
y = node.dimensions.height
y = node.dimensions.height
link.setAttribute('y', y.toString())
}
}
Expand Down Expand Up @@ -399,34 +444,46 @@ export default class Draw {
}
}

/**
* Check if given node has hidden children and render the eye icon if so
* @param {Node} node
*/
private checkForHiddenChildren(node: Node) {
Copy link
Member

Choose a reason for hiding this comment

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

this method does not only check, but actually hides or even removes Dom icons. That should be reflected in the name, as its otherwise confusing

if (node.hasHiddenChildNodes) {
this.setHiddenChildrenIcon(node)
} else {
this.removeHiddenChildrenIcon(node)
}
}

/**
* Update node name container (foreign object) dimensions.
* @param {Node} node
*/
private updateNodeNameContainer(node: Node) {
const name = node.getNameDOM(),
foreignObject: SVGForeignObjectElement = name?.parentNode as SVGForeignObjectElement
foreignObject: SVGForeignObjectElement = name?.parentNode as SVGForeignObjectElement

const [width, height]: number[] = (() => {
if (!this.browserIsFirefox()) {
// Default case
// Text is rendered based on needed width and height
// works well at least for chrome and safari
name.style.setProperty('width', 'auto')
name.style.setProperty('height', 'auto')
return [name.clientWidth, name.clientHeight]
// Default case
// Text is rendered based on needed width and height
// works well at least for chrome and safari
name.style.setProperty('width', 'auto')
name.style.setProperty('height', 'auto')
return [name.clientWidth, name.clientHeight]
} else {
// More recent versions of firefox seem to render too late to actually fetch the width and height of the dom element.
// In these cases, try to approximate height and width before rendering.
name.style.setProperty('width', '100%')
name.style.setProperty('height', '100%')
// split by line break
const linesByLineBreaks = name.textContent.split(/\r?\n|\r|\n/g)
// take longest line as width, when no lines are present use 1 as length
const width = Math.max(...linesByLineBreaks.map((line: string) => line.length), 1)
// take number of lines as height factor
const height = linesByLineBreaks.length
return [width * node.font.size / 1.2, height * node.font.size * 1.2]
// More recent versions of firefox seem to render too late to actually fetch the width and height of the dom element.
// In these cases, try to approximate height and width before rendering.
name.style.setProperty('width', '100%')
name.style.setProperty('height', '100%')
// split by line break
const linesByLineBreaks = name.textContent.split(/\r?\n|\r|\n/g)
// take longest line as width, when no lines are present use 1 as length
const width = Math.max(...linesByLineBreaks.map((line: string) => line.length), 1)
// take number of lines as height factor
const height = linesByLineBreaks.length
return [width * node.font.size / 1.2, height * node.font.size * 1.2]
}
})().map((value: number) => Math.max(value, 25))

Expand Down
3 changes: 3 additions & 0 deletions teammapper-frontend/mmp/src/map/handlers/export.ts
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,9 @@ export default class Export {
clone.setAttribute('transform', 'translate(0,0)')
svg.appendChild(clone)

// Remove any text related to Material Icons
d3.select(clone).selectAll('text.material-icons').text('')

// convert all foreignObjects to native svg text to ensure better compatibility with svg readers
d3.select(clone).selectAll('foreignObject').nodes().forEach((fo: HTMLElement) => {
const parent = fo.parentElement
Expand Down
36 changes: 31 additions & 5 deletions teammapper-frontend/mmp/src/map/handlers/history.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import Map from '../map'
import Node, {Colors, Coordinates, ExportNodeProperties, Font, Image, Link, NodeProperties} from '../models/node'
import {Event} from './events'
import Node, { Colors, Coordinates, ExportNodeProperties, Font, Image, Link, NodeProperties } from '../models/node'
import { Event } from './events'
import Log from '../../utils/log'
import Utils from '../../utils/utils'
import { DefaultNodeValues } from '../options'
Expand Down Expand Up @@ -54,17 +54,40 @@ export default class History {

this.save()

if(notifyWithEvent) this.map.events.call(Event.create, this.map.dom)
if (notifyWithEvent) this.map.events.call(Event.create, this.map.dom)
} else if (this.checkSnapshotStructure(snapshot)) {
const previousData = this.map.export.asJSON()

// Find all nodes where we've set hasHiddenChildNodes in the previous map
Copy link
Member

Choose a reason for hiding this comment

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

maybe apply refactoring: extract method, as this is a lot of logic only dedicating to the hiding of nodes.

const nodesWithHiddenChildren = previousData.filter(node => node.hasHiddenChildNodes)

// This method will recursively hide all children of children until none are left
const hideChildNodes = (parentId: string) => snapshot.filter(node => node.parent === parentId).forEach(node => {
node.hidden = true
hideChildNodes(node.id)
})

snapshot.forEach(snapshotNode => {
const nodeWithHiddenChildren = nodesWithHiddenChildren.find(x => snapshotNode.id === x.id)

if (nodeWithHiddenChildren) {
snapshotNode.hasHiddenChildNodes = true

// We need to iterate through the snapshot instead of using this.map.nodes.nodeChildren() to see if we need to set hidden attributes as the latter will not have new nodes added yet
snapshot.filter(node => node.parent === snapshotNode.id).forEach(node => {
node.hidden = true
hideChildNodes(node.id)
})
}
})

this.redraw(snapshot)

this.map.zoom.center('position', 0)

this.save()

if(notifyWithEvent) this.map.events.call(Event.create, this.map.dom, { previousMap: previousData })
if (notifyWithEvent) this.map.events.call(Event.create, this.map.dom, { previousMap: previousData })
} else {
Log.error('The snapshot is not correct')
}
Expand Down Expand Up @@ -99,6 +122,7 @@ export default class History {
}

this.snapshots.push(this.getSnapshot())

this.index++
}

Expand Down Expand Up @@ -135,13 +159,15 @@ export default class History {
link: Utils.cloneObject(mergedProperty.link) as Link,
locked: mergedProperty.locked,
detached: mergedProperty.detached,
hidden: mergedProperty.hidden,
hasHiddenChildNodes: mergedProperty.hasHiddenChildNodes,
isRoot: mergedProperty.isRoot
}

const node: Node = new Node(properties)
this.map.nodes.setNode(node.id, node)

if(mergedProperty.isRoot) this.map.rootId = mergedProperty.id
if (mergedProperty.isRoot) this.map.rootId = mergedProperty.id
})

this.map.draw.clear()
Expand Down
Loading
Loading