Make xpath default selection

SeleniumHQ / selenium-ide

Open Source record and playback test automation for the web.

https://selenium.dev/selenium-ide/

Apache License 2.0

2.73k stars 740 forks source link

Make xpath default selection #1716

Closed Mohammed-Fayaz closed 2 months ago

Mohammed-Fayaz commented 9 months ago

Thanks for contributing to the Selenium IDE! A PR well described will help maintainers to quickly review and merge it

Before submitting your PR, please check our contributing guidelines. Avoid large PRs, help reviewers by making them as simple and short as possible.

Description

Motivation and Context

Types of changes

[ ] Bug fix (non-breaking change which fixes an issue)
[ ] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to change)

Checklist

[ ] I have read the contributing document.
[ ] My change requires a change to the documentation.
[ ] I have updated the documentation accordingly.
[ ] I have added tests to cover my changes.
[ ] All new and existing tests passed.

CLAassistant commented 9 months ago

All committers have signed the CLA.

toddtarsi commented 9 months ago

@Mohammed-Fayaz - Absolutely not. This would force the most unrecommended selector types to the top.

I understand this is good for your use case. To better serve that, we need to get selector prioritization live, so you can put xpath:innertext as your number one. But no way is it going to be chosen over data-testid by default.

Mohammed-Fayaz commented 9 months ago

Of course @toddtarsi. We should ideally leave the prioritization up to the user. May be we can introduce an option to select the preference somewhere? I can take that change up.

toddtarsi commented 9 months ago

@Mohammed-Fayaz - Absolutely. I'll draft out how that can be done, but unfortunately, I won't be able to do so for about 12 hours. Once I'm clear, I'll really dive into it, so the next chance you have, there will be clear direction. There are some hooks for this stuff already, and this is something many people have asked for. Thank you for offering to help take up this feature. I'd be happy to work with you to help it succeed.

toddtarsi commented 9 months ago

@Mohammed-Fayaz - It's not a draft per se, but this basically wires up a first pass of the backend stuff for the selector reordering:

https://github.com/SeleniumHQ/selenium-ide/pull/1719

It still needs a frontend component nested here:

packages/selenium-ide/src/browser/windows/ProjectEditor/tabs/Project/SystemSettings.tsx

Basically, this component will be a drag and drop list that reads off of session.project.state.locators. When this list is reordered, it will call window.sideAPI.recorder.onLocatorOrderChanged.dispatchEvent(newLocatorsList). We use react-dnd under the surface for our drag and drop lists, and it has a pretty simple react api. You can probably get there working off this component as an example:

packages/selenium-ide/src/browser/components/ReorderableList.tsx

Mohammed-Fayaz commented 9 months ago

Looks good @toddtarsi. Will take a crack at it sometime in the next week.

toddtarsi commented 6 months ago

@Mohammed-Fayaz - I'm closing this PR in a week if you're not still working on this. Thanks for looking at the codebase!