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

UI: Double clicking on group propagating the checked state #4179

Merged
merged 4 commits into from
Jun 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
61 changes: 57 additions & 4 deletions assets/src/components/Treeview.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,13 @@ export default class Treeview extends HTMLElement {
constructor() {
super();
this._itemNameSelected;
this._clickTimestamp;
}

connectedCallback() {

this._onChange = () => {
if (this._freeze) return;
render(this._rootTemplate(mainLizmap.state.layerTree), this);
};

Expand Down Expand Up @@ -96,10 +98,14 @@ export default class Treeview extends HTMLElement {
}
<div class="${layer.checked ? 'checked' : ''} ${layer.type} ${layer.name === this._itemNameSelected ? 'selected' : ''}">
<div class="loading ${layer.loadStatus === MapLayerLoadStatus.Loading ? 'spinner' : ''}"></div>
<input type="checkbox" class="${parent.mutuallyExclusive ? 'rounded-checkbox' : ''}" id="node-${layer.name}" .checked=${layer.checked} @click=${() => layer.checked = !layer.checked} >
<input type="checkbox"
class="${parent.mutuallyExclusive ? 'rounded-checkbox' : ''}"
id="node-${layer.name}"
.checked=${layer.checked}
@click=${() => layer.checked = !layer.checked} >
<div class="node ${layer.isFiltered ? 'filtered' : ''}">
<img class="legend" src="${layer.icon}">
<label for="node-${layer.name}">${layer.layerConfig.title}</label>
<label for="node-${layer.name}" >${layer.layerConfig.title}</label>
<div class="layer-actions">
<a href="${this._createDocLink(layer.name)}" target="_blank" title="${lizDict['tree.button.link']}">
<i class="icon-share"></i>
Expand Down Expand Up @@ -131,10 +137,19 @@ export default class Treeview extends HTMLElement {
<div class="${group.checked ? 'checked' : ''} ${group.type} ${group.name === this._itemNameSelected ? 'selected' : ''}">
${mainLizmap.initialConfig.options.hideGroupCheckbox
? ''
: html`<input type="checkbox" class="${parent.mutuallyExclusive ? 'rounded-checkbox' : ''}" id="node-${group.name}" .checked=${group.checked} @click=${() => group.checked = !group.checked} >`
: html`<input type="checkbox" class="${parent.mutuallyExclusive ? 'rounded-checkbox' : ''}"
id="node-${group.name}"
.checked=${group.checked}
@click=${(evt) => this._clickItem(evt, group)}
@dblclick=${() => this._dblclickItem(group)} >`
}
<div class="node ${group.isFiltered ? 'filtered' : ''}">
<label for="node-${group.name}">${group.layerConfig.title}</label>
${mainLizmap.initialConfig.options.hideGroupCheckbox
? html`<label for="node-${group.name}" >${group.layerConfig.title}</label>`
: html`<label
for="node-${group.name}"
@dblclick=${() => this._dblclickItem(group)} } >${group.layerConfig.title}</label>`
}
<div class="layer-actions">
<a href="${this._createDocLink(group.name)}" target="_blank" title="${lizDict['tree.button.link']}">
<i class="icon-share"></i>
Expand Down Expand Up @@ -231,6 +246,44 @@ export default class Treeview extends HTMLElement {
return true;
}

_clickItem(evt, item) {
// Freeze or dblclick received
if (this._freeze || evt.detail > 1) {
// Force input element to keep checked status
evt.currentTarget.checked = item.checked;
return false;
}

// It is much more end2end test purpose
// a playwright dblclick is 2 clicks with detail 0
// and the dblclick which is a click with detail 2
if (this._clickTimestamp && evt.timeStamp - this._clickTimestamp < 1) {
// Force input element to keep checked status
evt.currentTarget.checked = item.checked;
return false;
}
this._clickTimestamp = evt.timeStamp;

item.checked = !item.checked;
return false;
}

_dblclickItem(item) {
if (item.type != 'group') {
rldhont marked this conversation as resolved.
Show resolved Hide resolved
return false;
}

if (this._freeze) {
return false;
}

this._freeze = true;
item.propagateCheckedState(item.checked);
this._freeze = false;
this._onChange();
return false;
}

_createDocLink(layerName) {
let url = lizMap.config.layers?.[layerName]?.link;

Expand Down
26 changes: 23 additions & 3 deletions assets/src/modules/state/LayerTree.js
Original file line number Diff line number Diff line change
Expand Up @@ -353,9 +353,29 @@ export class LayerTreeGroupState extends LayerTreeItemState {
}
}

/**
* Propagate throught tree item the new checked state
* @param {boolean} val The new checked state
* @returns {boolean} the new checked state
*/
propagateCheckedState(val) {
for (const item of this._items) {
if (item.type == 'group') {
item.propagateCheckedState(val);
} else {
item.checked = val;
}
if (item.checked && this.mutuallyExclusive) {
break;
}
}
this.checked = val;
return this.checked;
}

/**
* Find layer names
* @returns {string[]} The layer names of all tree layers
* @returns {string[]} List of layer names
*/
findTreeLayerNames() {
let names = []
Expand All @@ -371,7 +391,7 @@ export class LayerTreeGroupState extends LayerTreeItemState {

/**
* Find layer items
* @returns {LayerTreeLayerState[]} The tree layer states of all tree layers
* @returns {LayerTreeLayerState[]} List of tree layers (not tree groups)
*/
findTreeLayers() {
let items = []
Expand All @@ -387,7 +407,7 @@ export class LayerTreeGroupState extends LayerTreeItemState {

/**
* Find layer and group items
* @returns {LayerTreeLayerState[]} All tThe tree layer and tree group states
* @returns {LayerTreeLayerState[]} List of tree layers and tree groups
*/
findTreeLayersAndGroups() {
let items = []
Expand Down
44 changes: 22 additions & 22 deletions assets/src/modules/state/MapLayer.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,63 +80,63 @@ export class MapItemState extends EventDispatcher {
layerItemState.addListener(this.dispatch.bind(this), 'layer.filter.token.changed');
}
}

/**
* Config layers
* Map item name
* @type {string}
*/
get name() {
return this._layerItemState.name;
}

/**
* Config layers
* Map item type
* @type {string}
*/
get type() {
return this._type;
}

/**
* the layer tree item level
* the layer item level
* @type {number}
*/
get level() {
return this._layerItemState.level;
}

/**
* WMS layer name
* WMS item name
* @type {?string}
*/
get wmsName() {
return this._layerItemState.wmsName;
}

/**
* WMS layer title
* WMS item title
* @type {string}
*/
get wmsTitle() {
return this._layerItemState.wmsTitle;
}

/**
* WMS layer Geographic Bounding Box
* WMS item Geographic Bounding Box
* @type {?LayerGeographicBoundingBoxConfig}
*/
get wmsGeographicBoundingBox() {
return this._layerItemState.wmsGeographicBoundingBox;
}

/**
* WMS layer Bounding Boxes
* WMS item Bounding Boxes
* @type {LayerBoundingBoxConfig[]}
*/
get wmsBoundingBoxes() {
return this._layerItemState.wmsBoundingBoxes;
}


/**
* WMS Minimum scale denominator
* If the minimum scale denominator is not defined: -1 is returned
Expand All @@ -150,7 +150,7 @@ export class MapItemState extends EventDispatcher {
}

/**
* WMS layer maximum scale denominator
* WMS Maximum scale denominator
* If the maximum scale denominator is not defined: -1 is returned
* If the WMS layer is a group, the maximum scale denominator is the largest of the layers in the group
* @type {number}
Expand All @@ -160,23 +160,23 @@ export class MapItemState extends EventDispatcher {
}

/**
* Layer tree item is checked
* Map item is checked
* @type {boolean}
*/
get checked() {
return this._layerItemState.checked;
}

/**
* Set layer tree item is checked
* Set map item is checked
* @type {boolean}
*/
set checked(val) {
this._layerItemState.checked = val;
}

/**
* Layer tree item is visible
* Map item is visible
* It depends on the parent visibility
* @type {boolean}
*/
Expand All @@ -185,15 +185,15 @@ export class MapItemState extends EventDispatcher {
}

/**
* Layer tree item opacity
* Map item opacity
* @type {number}
*/
get opacity() {
return this._layerItemState.opacity;
}

/**
* Set layer tree item opacity
* Set map item opacity
* @type {number}
*/
set opacity(val) {
Expand All @@ -210,7 +210,7 @@ export class MapItemState extends EventDispatcher {

/**
* Lizmap layer item state
* @type {?LayerConfig}
* @type {?LayerItemState}
*/
get itemState() {
return this._layerItemState;
Expand Down Expand Up @@ -526,19 +526,19 @@ export class MapLayerState extends MapItemState {
}

/**
* set if the map layer is loaded in a single ImageWMS layer or not
* @param {boolean} val
* vector layer is loaded in a single layer ImageLayer or not
* @type {boolean}
*/
set singleWMSLayer(val){
this._singleWMSLayer = val;
get singleWMSLayer(){
return this._singleWMSLayer;
}

/**
* vector layer is loaded in a single layer ImageLayer or not
* set if the map layer is loaded in a single ImageWMS layer or not
* @type {boolean}
*/
get singleWMSLayer(){
return this._singleWMSLayer;
set singleWMSLayer(val){
this._singleWMSLayer = val;
}

/**
Expand Down
1 change: 1 addition & 0 deletions lizmap/www/assets/css/map.css
Original file line number Diff line number Diff line change
Expand Up @@ -3004,6 +3004,7 @@ lizmap-treeview input.rounded-checkbox:checked {

lizmap-treeview .group label {
font-weight: bold;
user-select: none;
}

lizmap-treeview label {
Expand Down
69 changes: 69 additions & 0 deletions tests/end2end/playwright/treeview.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,75 @@ test.describe('Treeview', () => {
test('displays "title" defined in Lizmap plugin', async ({ page }) => {
await expect(page.getByTestId('tramway_lines').locator('label')).toHaveText('Tramway lines');
});

test('double clicking', async ({ page }) => {
// All group1 is checked
await expect(page.locator('#node-group1')).toBeChecked();
await expect(page.locator('#node-sub-group1')).toBeChecked();
await expect(page.locator('#node-subdistricts')).toBeChecked();
// Unchecked all group1 by double clicking the label
await page.getByText('group1', { exact: true }).dblclick();
// All group1 is not checked
await expect(page.locator('#node-group1')).not.toBeChecked();
await expect(page.locator('#node-sub-group1')).not.toBeChecked();
await expect(page.locator('#node-subdistricts')).not.toBeChecked();
// Checked all group1 by double clicking the input
await page.getByLabel('group1', { exact: true }).dblclick();
// All group1 is checked
await expect(page.locator('#node-group1')).toBeChecked();
await expect(page.locator('#node-sub-group1')).toBeChecked();
await expect(page.locator('#node-subdistricts')).toBeChecked();

// Click to uncheck group1
await page.getByLabel('group1', { exact: true }).click();
// Only group1 is not checked
await expect(page.locator('#node-group1')).not.toBeChecked();
await expect(page.locator('#node-sub-group1')).toBeChecked();
await expect(page.locator('#node-subdistricts')).toBeChecked();

// Double clicking sub-group1 does not change the group1 checked state
// Because it because unchecked and group1 is already unchecked
await page.getByLabel('sub-group1', { exact: true }).dblclick();
await expect(page.locator('#node-group1')).not.toBeChecked();
await expect(page.locator('#node-sub-group1')).not.toBeChecked();
await expect(page.locator('#node-subdistricts')).not.toBeChecked();

// Double clicking sub-group1 changes the group1 checked state
await page.getByLabel('sub-group1', { exact: true }).dblclick();
await expect(page.locator('#node-group1')).toBeChecked();
await expect(page.locator('#node-sub-group1')).toBeChecked();
await expect(page.locator('#node-subdistricts')).toBeChecked();

// Verify the status of mutually exclusive group
await expect(page.getByLabel('group with space in name and shortname defined')).toBeChecked();
await expect(page.locator('#node-quartiers')).toBeChecked();
await expect(page.locator('#node-shop_bakery_pg')).not.toBeChecked();
// Unchecked all mutually exclusive group by double clicking the label
await page.getByText('group with space in name and shortname defined').dblclick();
await expect(page.getByLabel('group with space in name and shortname defined')).not.toBeChecked();
await expect(page.locator('#node-quartiers')).not.toBeChecked();
await expect(page.locator('#node-shop_bakery_pg')).not.toBeChecked();
// Checked all mutually exclusive group by double clicking the label, only the first child is clicked
await page.getByLabel('group with space in name and shortname defined').dblclick();
await expect(page.getByLabel('group with space in name and shortname defined')).toBeChecked();
await expect(page.locator('#node-quartiers')).toBeChecked();
await expect(page.locator('#node-shop_bakery_pg')).not.toBeChecked();
// switch visibility in mutually exclusive group
await page.locator('#node-shop_bakery_pg').click();
await expect(page.getByLabel('group with space in name and shortname defined')).toBeChecked();
await expect(page.locator('#node-quartiers')).not.toBeChecked();
await expect(page.locator('#node-shop_bakery_pg')).toBeChecked();
// Unchecked all mutually exclusive group by double clicking the label
await page.getByText('group with space in name and shortname defined').dblclick();
await expect(page.getByLabel('group with space in name and shortname defined')).not.toBeChecked();
await expect(page.locator('#node-quartiers')).not.toBeChecked();
await expect(page.locator('#node-shop_bakery_pg')).not.toBeChecked();
// Checked all mutually exclusive group by double clicking the label, only the first child is clicked
await page.getByLabel('group with space in name and shortname defined').dblclick();
await expect(page.getByLabel('group with space in name and shortname defined')).toBeChecked();
await expect(page.locator('#node-quartiers')).toBeChecked();
await expect(page.locator('#node-shop_bakery_pg')).not.toBeChecked();
});
});

test.describe('Treeview mocked with "Hide checkboxes for groups" option', () => {
Expand Down
Loading