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

getCSSProperty.js #46

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

getCSSProperty.js #46

wants to merge 1 commit into from

Conversation

djbf
Copy link
Contributor

@djbf djbf commented Sep 3, 2020

No description provided.

Copy link
Contributor

@pat-hanbury pat-hanbury left a comment

Choose a reason for hiding this comment

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

🥳 :shipit:

//let selector = getElementByXpath(mablInputs.variables.user.selector_xpath);

// using a css selector
let selector = document.querySelector(mablInputs.variables.user.selector_css);
Copy link
Contributor

Choose a reason for hiding this comment

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

The result of querySelector isn't a selector, but rather an element. Would it make sense to change this variable name to element and the one below this to style?

[nit] - selector_css could be be selectorCss to match the camel casing used below.

// using a css selector
let selector = document.querySelector(mablInputs.variables.user.selector_css);

let element = getComputedStyle(selector);
Copy link
Contributor

Choose a reason for hiding this comment

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

so I'm curious, we don't need window.getComputedStyle here right? it seems to work fine in dev tools when I'm testing it without window. I don't much about how the scoping works for our javascript snippets. Would be an easy thing to test though.

callback(element.getPropertyValue(mablInputs.variables.user.css_property));
}

function getElementByXpath(path) {
Copy link
Contributor

Choose a reason for hiding this comment

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

good idea having this here, it's handy :-)

Base automatically changed from master to main March 5, 2021 23:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants