Setting "Opacity" as an attribute in <layer-> tag #issue667new #733
Conversation
prushforth
left a comment
prushforth
left a comment
There was a problem hiding this comment.
Although layerAttribute is a pretty good name, unless you are planning to test the other attributes of the layer element in this same file, I would focus the name the test layerOpacityAttribute.test.js, to be clear.
|
|
||
| test("Setting Opacity Attibute to Layer- Element", async () => { | ||
| let opacity_attribute_value = await page.$eval("body > mapml-viewer > layer-",(layer) => layer.getAttribute("opacity")); | ||
| if (opacity_attribute_value == null){ |
There was a problem hiding this comment.
I believe you want to use "===" mostly in JavaScript (strict equality). However, I believe that you can re-phrase the logic of this as:
if (opacity_attribute_value) {
...
}
| let opacity = await page.$eval("body > mapml-viewer > layer-", (layer) => layer.opacity); | ||
| // console.log(opacity); | ||
| return; | ||
| } |
There was a problem hiding this comment.
I prefer to (try to) maintain one style of code block
Generally I prefer
if (something) {
...
} else if (something_else) {
...
} else {
...
}
There was a problem hiding this comment.
Functions very well, AFAICT! Please make the style changes suggested! We can merge after that.
I think we could add an opacity attribute to the <map-extent> element now and you will be better able to understand what the impacts are, although we don't have a <map-extent> custom element yet (soon!).
|
Works very nicely! Since a new attribute has been added to the layer, you should also update the web-map-doc to document the change (https://maps4html.org/web-map-doc/docs/layers/layer/#attributes). The API for the layer's opacity has already been documented here. |
|
Looks good @Jacky0299! Feel free to "Squash and merge" when ready. Good work! You can also merge the documentation, once this is merged. |
3 participants
closes #667