Skip to content

use property instead of atom to get index of option element #12662

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

Closed
wants to merge 4 commits into from

Conversation

titusfortner
Copy link
Member

Description

I've said I don't want to update code in the support module, but this was changed in Ruby & Java, but not Python, .NET or JS.

Motivation and Context

  1. Much more performant
  2. Probably fixes whatever is going on in [🐛 Bug]: selenium seems to override _, breaking another package #12659

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)

@titusfortner titusfortner added C-py Python Bindings C-dotnet .NET Bindings C-nodejs JavaScript Bindings labels Sep 1, 2023
@titusfortner titusfortner changed the title use property instead of atom to get index of element use property instead of atom to get index of option element Sep 1, 2023
@codecov-commenter
Copy link

codecov-commenter commented Sep 16, 2023

Codecov Report

Patch coverage has no change and project coverage change: -0.01% ⚠️

Comparison is base (609b43c) 56.98% compared to head (8f24c56) 56.97%.

❗ Current head 8f24c56 differs from pull request most recent head 90502a0. Consider uploading reports for the commit 90502a0 to get more accurate results

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##            trunk   #12662      +/-   ##
==========================================
- Coverage   56.98%   56.97%   -0.01%     
==========================================
  Files          86       86              
  Lines        5331     5330       -1     
  Branches      193      193              
==========================================
- Hits         3038     3037       -1     
  Misses       2100     2100              
  Partials      193      193              
Files Changed Coverage Δ
py/selenium/webdriver/support/select.py 80.00% <0.00%> (-0.17%) ⬇️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@titusfortner titusfortner force-pushed the select_property branch 3 times, most recently from 708e8b6 to 2eed0c5 Compare September 17, 2023 01:24
@titusfortner
Copy link
Member Author

Fun fact, the existing JavaScript code does not allow you to select by index with a number, only with a string.

@titusfortner
Copy link
Member Author

This should also change multiple to use getDomAttribute instead of getAttribute. I'm putting into draft. Low priority

@titusfortner titusfortner marked this pull request as draft September 17, 2023 17:36
@titusfortner
Copy link
Member Author

This didn't solve the issue I thought it did, and I don't really want to deal with it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-dotnet .NET Bindings C-nodejs JavaScript Bindings C-py Python Bindings
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants