Fix "false" tooltip on access fields (fixes #12064)#12095
Fix "false" tooltip on access fields (fixes #12064)#12095Ashish-Ranjan-jpg wants to merge 5 commits intoopenstreetmap:developfrom
Conversation
|
Could you link / check the code for other fields eg the combo field and compare this part of the code. We should use the same style for both. |
Refactor: Use ternary operator for access field tooltip title Co-authored-by: Tobias <t@tobiasjordans.de>
|
Checked other fields and they seems to follow the same styling. Updated the radio.js to use |
|
@tordans I’ve addressed the suggested things, could you please take another look on it. |
tordans
left a comment
tordans
left a comment
There was a problem hiding this comment.
Can only comment some more opinion on this. I don't do full reviews and cannot do merges here.
| } | ||
| return null; | ||
| }) | ||
| .attr('title', d => d.isMixed ? t('inspector.unshared_value_tooltip') : (!['yes', 'no'].includes(d.state) ? d.state : undefined)) |
There was a problem hiding this comment.
We don't use arrow functions (yet); not sure if we want to start here and now.
(There is discussion on if/when to use them in the future in a different thread.)
For me ternary operators work great of one level but the code before with two conditions was easier to read/understand.
There was a problem hiding this comment.
Changed the arrow function back to the regular functions .
Yes, using ternary operator there made it bit uneasy to read, so changed it back with two conditions.
Please let me know if everything is ok now.
| @@ -97,7 +97,7 @@ | |||
| "svg-sprite": "2.0.4", | |||
| "typescript": "^5.9.3", | |||
| "typescript-eslint": "^8.56.0", | |||
| "vite": "^8.0.1", | |||
| "vite": "^7.3.1", | |||
There was a problem hiding this comment.
Looks like we are on vite 8 now, interesting https://github.com/openstreetmap/iD/blob/develop/package.json#L141
But this package lock change should not be in the PR AFAIK and IMO.
There was a problem hiding this comment.
Staged the package-lock file unintentionally, reset it back to upstream/develop, now it is not in the "files changed" of this pr.
2 participants
This PR fixes a bug where access fields (e.g., access,
motor_vehicle,foot) show a literal string"false"in their tooltip when a single value is set.The Problem
In modules/ui/fields/access.js, the logic for the
titleattribute used a logical&&expression that returned a booleanfalseif the tag value was not an array (i.e., not a mixed selection). D3 then rendered this boolean as the string"false"in the HTMLtitleattribute.The Solution
Updated the logic to return
undefinedinstead offalsein those cases. Returningundefinedcorrectly instructs D3 and the browser not to render a tooltip for single-selection tags, which matches the behavior of other fields in the iD editor.Visuals
Testing
http://localhost:8080.npx eslint modules/ui/fields/access.jsand confirmed the file is clean.fixes #12064