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

Fix broken showLabel=null (on hover) #2490

Merged
merged 3 commits into from
Feb 11, 2025
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
45 changes: 23 additions & 22 deletions umap/static/umap/js/modules/form/builder.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ export class Form extends Utils.WithEvents {
try {
value = value[sub]
} catch {
console.log(field)
console.debug(field)
}
}
return value
Expand Down Expand Up @@ -142,49 +142,50 @@ export class MutatingForm extends Form {
slugKey: 'PropertyInput',
labelKey: 'PropertyInput',
}
for (const [key, schema] of Object.entries(SCHEMA)) {
if (schema.type === Boolean) {
if (schema.nullable) schema.handler = 'NullableChoices'
else schema.handler = 'Switch'
} else if (schema.type === 'Text') {
schema.handler = 'Textarea'
} else if (schema.type === Number) {
if (schema.step) schema.handler = 'Range'
else schema.handler = 'IntInput'
} else if (schema.choices) {
const text_length = schema.choices.reduce(
for (const [key, defaults] of Object.entries(SCHEMA)) {
const properties = Object.assign({}, defaults)
if (properties.type === Boolean) {
if (properties.nullable) properties.handler = 'NullableChoices'
else properties.handler = 'Switch'
} else if (properties.type === 'Text') {
properties.handler = 'Textarea'
} else if (properties.type === Number) {
if (properties.step) properties.handler = 'Range'
else properties.handler = 'IntInput'
} else if (properties.choices) {
const text_length = properties.choices.reduce(
(acc, [_, label]) => acc + label.length,
0
)
// Try to be smart and use MultiChoice only
// for choices where labels are shorts…
if (text_length < 40) {
schema.handler = 'MultiChoice'
properties.handler = 'MultiChoice'
} else {
schema.handler = 'Select'
schema.selectOptions = schema.choices
properties.handler = 'Select'
properties.selectOptions = properties.choices
}
} else {
switch (key) {
case 'color':
case 'fillColor':
schema.handler = 'ColorPicker'
properties.handler = 'ColorPicker'
break
case 'iconUrl':
schema.handler = 'IconUrl'
properties.handler = 'IconUrl'
break
case 'licence':
schema.handler = 'LicenceChooser'
properties.handler = 'LicenceChooser'
break
}
}

if (customHandlers[key]) {
schema.handler = customHandlers[key]
properties.handler = customHandlers[key]
}
// Input uses this key for its type attribute
delete schema.type
this.defaultProperties[key] = schema
delete properties.type
this.defaultProperties[key] = properties
}
}

Expand All @@ -202,7 +203,7 @@ export class MutatingForm extends Form {
getTemplate(helper) {
let template
if (helper.properties.inheritable) {
const extraClassName = helper.get(true) === undefined ? ' undefined' : ''
const extraClassName = this.getter(helper.field) === undefined ? ' undefined' : ''
template = `
<div class="umap-field-${helper.name} formbox inheritable${extraClassName}">
<div class="header" data-ref=header>
Expand Down
12 changes: 7 additions & 5 deletions umap/static/umap/js/modules/form/fields.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,11 +80,13 @@ class BaseElement {
this.input.value = ''
}

get(own) {
if (!this.properties.inheritable || own) return this.builder.getter(this.field)
get() {
if (!this.properties.inheritable) return this.builder.getter(this.field)
const path = this.field.split('.')
const key = path[path.length - 1]
return this.obj.getOption(key) || SCHEMA[key]?.default
const value = this.obj.getOption(key)
if (value === undefined) return SCHEMA[key]?.default
return value
}

toHTML() {
Expand Down Expand Up @@ -1164,7 +1166,7 @@ Fields.MultiChoice = class extends BaseElement {

Fields.TernaryChoices = class extends Fields.MultiChoice {
getDefault() {
return 'null'
return null
}

toJS() {
Expand Down Expand Up @@ -1195,7 +1197,7 @@ Fields.NullableChoices = class extends Fields.TernaryChoices {
this.properties.choices || [
[true, translate('always')],
[false, translate('never')],
['null', translate('hidden')],
[null, translate('hidden')],
]
)
}
Expand Down
2 changes: 1 addition & 1 deletion umap/static/umap/js/modules/schema.js
Original file line number Diff line number Diff line change
Expand Up @@ -450,7 +450,7 @@ export const SCHEMA = {
choices: [
[true, translate('always')],
[false, translate('never')],
['null', translate('on hover')],
[null, translate('on hover')],
],
},
slideshow: {
Expand Down
16 changes: 16 additions & 0 deletions umap/tests/integration/test_edit_map.py
Original file line number Diff line number Diff line change
Expand Up @@ -210,3 +210,19 @@ def test_sortkey_impacts_datalayerindex(map, live_server, page):
assert "Z First" == first_listed_feature.text_content()
assert "Y Second" == second_listed_feature.text_content()
assert "X Third" == third_listed_feature.text_content()


def test_hover_tooltip_setting_should_be_persistent(live_server, map, page):
map.settings["properties"]["showLabel"] = None
map.edit_status = Map.ANONYMOUS
map.save()
page.goto(f"{live_server.url}{map.get_absolute_url()}?edit")
page.get_by_role("button", name="Map advanced properties").click()
page.get_by_text("Default interaction options").click()
expect(page.get_by_text("on hover")).to_be_visible()
expect(page.locator(".umap-field-showLabel")).to_match_aria_snapshot("""
- text: Display label
- button "clear"
- text: always never on hover
""")
expect(page.locator(".umap-field-showLabel input[value=null]")).to_be_checked()
10 changes: 10 additions & 0 deletions umap/tests/integration/test_view_marker.py
Original file line number Diff line number Diff line change
Expand Up @@ -181,3 +181,13 @@ def test_only_visible_markers_are_added_to_dom(live_server, map, page):
)
expect(markers).to_have_count(1)
expect(tooltips).to_have_count(1)


def test_should_display_tooltip_on_hover(live_server, map, page, bootstrap):
map.settings["properties"]["showLabel"] = None
map.settings["properties"]["labelKey"] = "Foo {name}"
map.save()
page.goto(f"{live_server.url}{map.get_absolute_url()}")
expect(page.get_by_text("Foo test marker")).to_be_hidden()
page.locator(".leaflet-marker-icon").hover()
expect(page.get_by_text("Foo test marker")).to_be_visible()