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

issues_1189_1190: Fixed switching between locator types and validation icon #1202

Merged
merged 7 commits into from
Apr 26, 2023

Conversation

arslanbekova
Copy link
Member

No description provided.

@@ -1,22 +1,20 @@
import React from "react";
import { Spin, Tooltip } from "antd";
import Icon from "@ant-design/icons";
import CheckedEdited from "../assets/checked-edited.svg";
Copy link
Contributor

Choose a reason for hiding this comment

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

please, delete svg file as well

Copy link
Member Author

Choose a reason for hiding this comment

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

done

newValue.locator.customXpath = locator;
locatorType !== LocatorType.cssSelector
? (newValue.locator.customXpath = locator)
: (newValue.locator.cssSelector = locator);
Copy link
Contributor

Choose a reason for hiding this comment

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

please, use if. ternary looks confusing here

Copy link
Contributor

@MariiaNebesnova MariiaNebesnova Apr 26, 2023

Choose a reason for hiding this comment

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

or you can write smth like following:

newValue.locator = locatorType !== LocatorType.cssSelector
          ? { ...newValue.locator, customXpath: locator }
          :  { ...newValue.locator, cssSelector: locator };

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

const nodeSnapshot = document.evaluate(xPath, document, null, XPathResult.ORDERED_NODE_SNAPSHOT_TYPE, null);
const length = nodeSnapshot.snapshotLength;
const foundElement = nodeSnapshot.snapshotItem(0) as Element;
const foundElements = document.querySelectorAll(selector);
Copy link
Contributor

Choose a reason for hiding this comment

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

let's use querySelector, since there's no need in ...All

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

value === locatorType ? form.setFieldValue("locator", locatorField) : form.setFieldValue("locator", "");
} else {
form.setFieldValue("locator", getLocator(locator, value));
}
Copy link
Contributor

Choose a reason for hiding this comment

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

there if's need to be refactored. for example, extract them to functions.
also, code will be less verbose if you write form.setFieldValue("locator", result) once in the end, and only calculate result value above


const onFieldsChange = async (changedValues: any) => {
const [changedValue] = changedValues;
const isLocatorTypeChanged = changedValue?.name[0] === "locatorType";
Copy link
Contributor

Choose a reason for hiding this comment

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

what is the type of changedValues? Is there always will be inly one value? Using index can be unstable for cases when there are more that one item in array

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed

Copy link
Contributor

@MariiaNebesnova MariiaNebesnova left a comment

Choose a reason for hiding this comment

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

Thanks for code!

@arslanbekova arslanbekova merged commit cf05a00 into release_3.11 Apr 26, 2023
2 of 3 checks passed
@arslanbekova arslanbekova deleted the hotfix_1189 branch April 27, 2023 12:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants