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

FeatureRequest: _WD_ElementSelectAction() 'selectvalue' and 'selecttext' and 'selectindex' #168

Closed
mlipok opened this issue Jan 31, 2022 · 40 comments

Comments

@mlipok
Copy link
Contributor

mlipok commented Jan 31, 2022

As for now this is only a concept.

Func _WD_ElementSelectAction($sSession, $sSelectElement, $sCommand, $sParameter)
	......
	......
	......
	If $iErr = $_WD_ERROR_Success Then
		If $sNodeName = 'select' Then
			Switch $sCommand
				Case 'value'
					......
					......
					......
				Case 'options'
					......
					......
					......
				Case 'selectindex'
					$sScript = "arguments[0].selectedIndex = " & $sParameter & "; return true;"
					$sText = _WD_ExecuteScript($sSession, $sScript, __WD_JsonElement($sSelectElement), Default, $_WD_JSON_Value)
					$iErr = @error
					If $iErr = $_WD_ERROR_Success Then
						If $sText = True Then
							......
						EndIf
					EndIf
				Case 'selectvalue'
;~ 					$sScript = "var element = arguments[0]; for (let i = 0; i < element.options.length; i++) { if (element.options[i].value == '" & $sParameter & "') {element.selectedIndex = i; element.dispatchEvent(new Event('change')); return true;}}; return false;"
					$sScript = "var element = arguments[0]; for (let i = 0; i < element.options.length; i++) { if (element.options[i].value == '" & $sParameter & "') {element.selectedIndex = i; return true;}}; return false;"
					$sText = _WD_ExecuteScript($sSession, $sScript, __WD_JsonElement($sSelectElement), Default, $_WD_JSON_Value)
					$iErr = @error
					If $iErr = $_WD_ERROR_Success Then
						If $sText = True Then
							......
						EndIf
					EndIf
				Case 'selecttext'
;~ 					$sScript = "var element = arguments[0]; for (let i = 0; i < element.options.length; i++) { if (element.options[i].text == '" & $sParameter & "') {element.selectedIndex = i; element.dispatchEvent(new Event('change')); return true;}}; return false;"
					$sScript = "var element = arguments[0]; for (let i = 0; i < element.options.length; i++) { if (element.options[i].text == '" & $sParameter & "') {element.selectedIndex = i; return true;}}; return false;"
					$sText = _WD_ExecuteScript($sSession, $sScript, __WD_JsonElement($sSelectElement), Default, $_WD_JSON_Value)
					$iErr = @error
					If $iErr = $_WD_ERROR_Success Then
						If $sText = True Then
							......
						EndIf
					EndIf

	......
	......
	......
EndFunc   ;==>_WD_ElementSelectAction

But what you think about ?

@mlipok mlipok changed the title FeatureRequest: _WD_ElementSelectAction() 'selectvalue' and 'selecttext' FeatureRequest: _WD_ElementSelectAction() 'selectvalue' and 'selecttext' and 'selectindex' Jan 31, 2022
@Danp2
Copy link
Owner

Danp2 commented Jan 31, 2022

@mlipok Some general thoughts in no particular order --

  • If this is implemented, the additional parameter would need to be optional
  • selectindex could be used to both get / set the element's selectedIndex property
  • Not sure that I see the value of using _WD_ExecuteScript in the other options. Can't this be done with some of the basic webdriver commands?

IIRC, _WD_ElementSelectAction was originally created to provide access to a Select element's details that weren't otherwise available. We should try to stick to that "mission" as we look at adding features.

@mlipok
Copy link
Contributor Author

mlipok commented Jan 31, 2022

IMHO _WD_ElementSelectAction() should have all features needed to do all stuff related to "HTMLSelectElement"

@Danp2
Copy link
Owner

Danp2 commented Jan 31, 2022

IMNSHO I disagree. 😄

HTMLSelectElement has many different PEMs and it would be unrealistic to expect _WD_ElementSelectAction to "do it all".

Can you demonstrate where there is a demand for these additional features?

@mlipok
Copy link
Contributor Author

mlipok commented Jan 31, 2022

HTMLSelectElement has many different PEMs and it would be unrealistic to expect _WD_ElementSelectAction to "do it all".

Yeah I agree that all are not required in this UDF but the main:

  • Get list of options - already exist OPTIONS
  • Get currently selected - partialy exist VALUE - there is no option to get Index of the selected Option
  • Select by ID
  • Select by Value
  • Select by Text

take a look on _IEFormElementOptionSelect

$sMode [optional] specifies search mode
    "byValue" = (Default) value of the option you wish to select
    "byText" = text of the option you wish to select
    "byIndex" = 0-based index of option you wish to select

The only one I propose is to have adequte features in WD UDF

@Danp2
Copy link
Owner

Danp2 commented Jan 31, 2022

take a look on _IEFormElementOptionSelect

This UDF already has a similar function (_WD_ElementOptionSelect), but it relies on the user providing the proper selector and it doesn't break down the individual modes.

IMO, if more "select" options are going to be added, they should be implemented here and not it _WD_ElementSelectAction.

@mlipok
Copy link
Contributor Author

mlipok commented Jan 31, 2022

I think that _WD_ElementOptionSelect is simple CLICK wrapper and has nothing to do with _WD_ElementSelectAction
Other problem is that _WD_ElementOptionSelect has inproper parameters to do the job.
_WD_ElementSelectAction is more accurate.

I trully do not understand why there are two separate functions for Select Element.

@Danp2
Copy link
Owner

Danp2 commented Jan 31, 2022

They deal with different element types, which is why there are two separate functions --

  • _WD_ElementOptionSelect is designed to find an Option element using a selector and then click it.

  • _WD_ElementSelectAction is designed to perform various actions on a previously located Select element.

Therefore, code for selecting an Option element doesn't belong in _WD_ElementSelectAction. However, it would be appropriate to add code dealing with selectedIndex since that's a Select element property.

@Danp2
Copy link
Owner

Danp2 commented Jan 31, 2022

_WD_ElementOptionSelect could be redesigned so that it behaves more like _IEFormElementOptionSelect, but we would need to decide if it was worth implementing another script breaking change.

@mlipok
Copy link
Contributor Author

mlipok commented Feb 1, 2022

  • _WD_ElementOptionSelect is designed to find an Option element using a selector and then click it.

So you must to know which option Element you want to operate.
For me this is click wrapper nothing more.

  • _WD_ElementSelectAction is designed to perform various actions on a previously located Select element.

Why not 'selecting' by performing search over all options on a previously located Select element ?
These are functionalities / actions consisting in searching all options and performing operations on them.

_WD_ElementSelectAction have suitable paramters to do that without any script breaking changes.

btw.
You must to have in mind that next step will be support for multi selection.
Doing all this stuff with JavaScript will be much faster.

@Danp2
Copy link
Owner

Danp2 commented Feb 1, 2022

So you must to know which option Element you want to operate.
For me this is click wrapper nothing more.

Technically, it's a wrapper for locating (_WD_FindElement) and clicking (_WD_ElementAction) an element. Here's a link to the original discussion that spawned this function.

Theoretically, we could rename this function to _WD_FindAndClickElement.

Why not 'selecting' by performing search over all options on a previously located Select element ?

I think you are starting to wear me down. 😆

My concern is that the implementation needs to make sense to me. Currently we have value and options, which both gather information. You want to add some commands that do the opposite, which is ok but I just need to be able to wrap my head around it.

I suggest that we add a single select command, where additional parameters / settings will control the type of selection (by index, id, name, etc) to be performed. Obviously we don't want to add a bunch of extra parameters. You already proposed adding $sParameter. Think about it and let me know if you come up with some other ideas.

Obviously, input from others is welcome here too. 👍

@mlipok
Copy link
Contributor Author

mlipok commented Feb 1, 2022

Why not 'selecting' by performing search over all options on a previously located Select element ?

I think you are starting to wear me down. 😆

Sorry for wearing you down. I can blame it on my poor English ;)

My concern is that the implementation needs to make sense to me.

Now you answer to my question. Thanks.

I suggest that we add a single select command, where additional parameters / settings will control the type of selection (by index, id, name, etc) to be performed. Obviously we don't want to add a bunch of extra parameters. You already proposed adding $sParameter. Think about it and let me know if you come up with some other ideas.

If we add a single select command where additional $sParameter will specify which type of search action will be taken, then we be in a need to add additionall paramter to specify desirable value (IndexNumber, name, text).

Thus I propos to specify 3 different command name which will lead to specific selection type (search and selection action), which will allow to add only one additional parameter which I call $sParameter to provide desirable value (IndexNumber, name, text).

Obviously, input from others is welcome here too. 👍

Absolutely right.
So let's wait for the comments of others.

@Danp2
Copy link
Owner

Danp2 commented Feb 1, 2022

Think about it and let me know if you come up with some other ideas.

@mlipok Still hoping that you will suggest some other alternatives that don't involve adding even more parameters. Here are a few (not necessarily good ones 😉 ) to get you started --

  • Change $sCommand to $aCommand; aCommand[0] would contain the primary command and any additional array entries would represent the additonal values need to perform the command.

  • Change $sCommand to $vCommand; if $vCommand` is an array, then apply the same logic in the above item

  • Change $sParameter to $aParameters where the array holds all necessary values to perform $sCommand

  • $sParameter could hold both the type and the search value with a delimiter separating the two items

@mlipok
Copy link
Contributor Author

mlipok commented Feb 1, 2022

Here's a link to the original discussion that spawned this function.

Just start analyzing. Will test this first.

@mlipok
Copy link
Contributor Author

mlipok commented Feb 2, 2022

Checked:

Local $v_Result = _WD_ElementOptionSelect($sSession, $_WD_LOCATOR_ByCSSSelector, '#MainContentForm_ddlAdvStatus.form-control option[value="8"]')
Local $v_Result = _WD_ElementOptionSelect($sSession, $_WD_LOCATOR_ByXPath, '//select[@id="MainContentForm_ddlAdvStatus"]/option[@value="8"]')
Local $v_Result = _WD_ElementOptionSelect($sSession, $_WD_LOCATOR_ByXPath, '//select[@id="MainContentForm_ddlAdvStatus"]/option[contains(text(), "Wszystkie wpisy, w tym wykreślone")]')

All this function works.

So selecting by Value and by Text is possible with _WD_ElementOptionSelect() but this was not well documented.
I plan to prepare examples and put them in these places:
https://www.autoitscript.com/wiki/WebDriver#FAQ
https://www.autoitscript.com/forum/topic/201537-webdriver-example-scripts-collection/

As to selecting by Index , my plan is to provide PR with this code concept:

Case 'selectedindex'
	$sScript = "arguments[0].selectedIndex = " & $sParameter
	$sText = _WD_ExecuteScript($sSession, $sScript, __WD_JsonElement($sSelectElement), Default, $_WD_JSON_Value)
	$iErr = @error
	If $iErr = $_WD_ERROR_Success Then
		If $sText = True Then
			......
		EndIf
	EndIf

What you think about ?

@Danp2
Copy link
Owner

Danp2 commented Feb 2, 2022

So selecting by Value and by Text is possible with _WD_ElementOptionSelect() but this was not well documented.

How would you propose to document it further besides providing examples?

I haven't looked into it, but you may be able to select by index using the same technique. If you get a chance, check it out and let me know what you find.

@mlipok
Copy link
Contributor Author

mlipok commented Feb 2, 2022

As to the example, take a look here:
https://www.autoitscript.com/forum/topic/201537-webdriver-example-scripts-collection/?do=findComment&comment=1496107

Do you think that this could be or not provided in wd_demo.au3 or the examples topic/thread in the forum is right place, or should I add them to the WiKi ?

you may be able to select by index using the same technique. If you get a chance, check it out and let me know what you find.

There is an implementation of this technique.

@mlipok
Copy link
Contributor Author

mlipok commented Feb 2, 2022

How would you propose to document it further besides providing examples?

For all occurance of:

;                  $sSelector    - Value to find

Proposal:

;                  $sSelector    - String or expression related to chosed $sStrategy which should navigate to the specific element.

for _WD_ElementOptionSelect()

Proposal:

; Remarks .......: You can choose Option by Index, Value, Text by providing proper XPath or CSSSelector expression as a $sSelector parameter

@Danp2
Copy link
Owner

Danp2 commented Feb 2, 2022

As to the example, take a look here

There are a few issues with this example --

  • It autoupdates Chromedriver, which I feel shouldn't occur without user approval
  • Your usage of _WD_Timeouts is incorrect

Do you think that this could be or not provided in wd_demo.au3 or the examples topic/thread in the forum is right place, or should I add them to the WiKi ?

IDK. I'm not sure that a separate demo is necessary. Maybe a reduced form of it could be incorporated into DemoElements. Maybe the wiki could have a separate page on CSS / xpath selectors with some examples.

@Danp2
Copy link
Owner

Danp2 commented Feb 2, 2022

$sSelector - String or expression related to chosed $sStrategy which should navigate to the specific element.

Lots of issues here IMO. Let's break it down into small chunks --

String or expression

I'm not sure what this means or how it improves the user's understanding of the parameter's usage. Please explain to me how adding the word "expression" helps here.

chosed $sStrategy

"chosed" is the wrong word here. Maybe you wanted "chosen", "indicated", or something similar.

which should navigate to the specific element

"navigate" is confusing here. I think of browser navigation unless you add something like "the HTML DOM" following the word navigate.

@mlipok
Copy link
Contributor Author

mlipok commented Feb 2, 2022

Please explain to me how adding the word "expression" helps here.

XPath stirnigs are expressions
https://developer.mozilla.org/en-US/docs/Web/XPath
XPath is mainly used in XSLT, but can also be used as a much more powerful way of navigating through the DOM of any XML-like language document using XPathExpression, such as HTML and SVG, instead of relying on the Document.getElementById() or Document.querySelectorAll() methods, the Node.childNodes properties, and other DOM Core features.

So I was thinking that this will be correct to use this word.

We could write it

;                  $sSelector    - String, Xpath expression or CSS Selectors related to choosed $sStrategy which should navigate to the specific element.

or simply

;                  $sSelector    - String related to choosen $sStrategy which should navigate to the specific element.

And after thinking about all yours other sugestion about corretness in using "English" (thanks) here is new proposal:

;                  $sSelector    - string related to choosen $sStrategy that should indicate how the WebDriver should traverse through the HTML DOM to the specified element.

@mlipok
Copy link
Contributor Author

mlipok commented Feb 2, 2022

As to the example, take a look here

There are a few issues with this example --

  • It autoupdates Chromedriver, which I feel shouldn't occur without user approval
  • Your usage of _WD_Timeouts is incorrect

Fixed , check the link again.

@mlipok
Copy link
Contributor Author

mlipok commented Feb 2, 2022

you may be able to select by index using the same technique. If you get a chance, check it out and let me know what you find.

What you think about my implementation this technique?

@mlipok
Copy link
Contributor Author

mlipok commented Feb 2, 2022

for _WD_ElementOptionSelect()

Proposal:

; Remarks .......: You can choose Option by Index, Value, Text by providing proper XPath or CSSSelector expression as a $sSelector parameter

So this also should be changed:

; Remarks .......: You can indicate specific <Option> element by Index, Value, Text by providing proper XPath or CSSSelector string as a $sSelector parameter

@Danp2
Copy link
Owner

Danp2 commented Feb 2, 2022

choosen

s/b "chosen" 😄

But that still seems to "wordy" to me. I would prefer something more succinct such as --

; $sSelector - Indicates how the WebDriver should traverse through the HTML DOM to locate the desired element(s).

To me, it's obvious that $sSelector needs to be in the format specificed in $sStrategy. However, this could be clarified in the remarks section if you think it necessary to state in the header.

@Danp2
Copy link
Owner

Danp2 commented Feb 2, 2022

; Remarks .......: You can indicate specific element by Index, Value, Text by providing proper XPath or CSSSelector string as a $sSelector parameter

Again, I feel this is self-explanatory. It's simply how webdriver works. Maybe this would be better suited for a "getting started" section of the wiki? 🤷‍♂️

@Danp2
Copy link
Owner

Danp2 commented Feb 2, 2022

What you think about my implementation this technique?

Please don't take this the wrong way, but I don't see anything "mind blowing" in that example. After all, you're just dynamically building CCS and xpath selectors, right? 🙂

@mlipok
Copy link
Contributor Author

mlipok commented Feb 2, 2022

Again, I feel this is self-explanatory. It's simply how webdriver works.

Okay. Understood.

Maybe this would be better suited for a "getting started" section of the wiki?

For me WiKi WD page is like a "getting started guide" especially:
https://www.autoitscript.com/wiki/WebDriver#Installation

So I feel that we should not create sub page, or sub section for "getting started"

@mlipok
Copy link
Contributor Author

mlipok commented Feb 2, 2022

Please don't take this the wrong way, but I don't see anything "mind blowing" in that example. After all, you're just dynamically building CCS and xpath selectors, right? 🙂

I was just asking is that what you was thinking about, when you proposed to use this technique ?

@mlipok
Copy link
Contributor Author

mlipok commented Feb 2, 2022

So I feel that we should not create sub page, or sub section for "getting started"

But maybe you meant to additionally show an example of simple concise code with comments?
Because wd_demo.au3 becomes a bit too complex for "wd_GettingStarted.au3"

@Danp2
Copy link
Owner

Danp2 commented Feb 2, 2022

So I feel that we should not create sub page, or sub section for "getting started"

Ok. Maybe a different name then? Something like "Beginner's guide to Webdriver selectors"

"wd_GettingStarted.au3"

FWIW, I don't think we need more source files at this stage.

I was just asking is that what you was thinking about, when you proposed to use this technique ?

Yes. My general thinking was that the UDF user needs to understand the various selectors; it isn't targeted at beginners, so we shouldn't need to provide separate functions for each selection type (id, name, etc) when a single function would suffice.

Granted, we since added some functions like _WD_GetElementById and _WD_GetElementByName, but I feel like that covers the basics.

@mlipok
Copy link
Contributor Author

mlipok commented Feb 2, 2022

"wd_GettingStarted.au3"

FWIW, I don't think we need more source files at this stage.

I was thinking about such example:

#include "wd_helper.au3"
#include "wd_capabilities.au3"

_Example()

Func _Example()
	# REMARK
	# This is not functional script
	# It only shows the concept how to use WebDriver UDF

	; If you want to download/update dirver the next line should be uncommented
;~ 	_WD_UpdateDriver('chrome')

	; specify used driver port parameters and other options
	_WD_Option('Driver', 'chromedriver.exe')
	_WD_Option('Port', 9515)
	_WD_Option('DriverParams', '--verbose --log-path="' & @ScriptDir & '\chrome.log"')

	; start the driver
	_WD_Startup()

	; create capabilites for session
	_WD_CapabilitiesStartup()
	_WD_CapabilitiesAdd('alwaysMatch', 'chrome')
	_WD_CapabilitiesAdd('w3c', True)
	_WD_CapabilitiesAdd('excludeSwitches', 'enable-automation')
	Local $sCapabilities = _WD_CapabilitiesGet()

	; create session with given Capabilities
	Local $WD_SESSION = _WD_CreateSession($sCapabilities)

	; navigate to some website
	Local $sURL = '******'
	_WD_Navigate($WD_SESSION, $sURL)

	; wait for loading process ends
	_WD_LoadWait($WD_SESSION, 1000)

	#Region - do your's stuff
	; for example find element
	Local $sElement = _WD_FindElement($WD_SESSION, $_WD_LOCATOR_ByXPath, "*****")
	; get text
	Local $sText = _WD_ElementAction($WD_SESSION, $sElement, 'text')
	ConsoleWrite($sText & @CRLF)
	; or click the element
	_WD_ElementAction($WD_SESSION, $sElement, 'click')
	#EndRegion - do your's stuff

	; on the end session should be deleted
	_WD_DeleteSession($WD_SESSION)

	; and driver should be closed
	_WD_Shutdown()
EndFunc   ;==>_Example

@mlipok
Copy link
Contributor Author

mlipok commented Feb 2, 2022

Yes. My general thinking was that the UDF user needs to understand the various selectors; it isn't targeted at beginners, so we shouldn't need to provide separate functions for each selection type (id, name, etc) when a single function would suffice.

Granted, we since added some functions like _WD_GetElementById and _WD_GetElementByName, but I feel like that covers the basics.

Ok now I understood what you mean about "Getting Started" on WiKi.
You was thinking about "How to deal with $sSelector's ?"

@Danp2
Copy link
Owner

Danp2 commented Feb 2, 2022

I was thinking about such example:

Maybe we can include something like this when we eventually get around to providing example code for the help file (see #133). For now, it will have to wait.

You was were thinking about "How to deal with $sSelector's ?"

Yes, something that gave a basic overview of selectors, the differences between xpath & css, and perhaps provided links to some other recommended reading.

@mlipok
Copy link
Contributor Author

mlipok commented Feb 2, 2022

SUMMARY:

As to selecting by Index , my plan is to provide PR with this code concept:

Case 'selectedindex'
	$sScript = "arguments[0].selectedIndex = " & $sParameter
	$sText = _WD_ExecuteScript($sSession, $sScript, __WD_JsonElement($sSelectElement), Default, $_WD_JSON_Value)
	$iErr = @error
	If $iErr = $_WD_ERROR_Success Then
		If $sText = True Then
			......
		EndIf
	EndIf

What you think about ?

Is this the only one thing which should be done as PR ?

Do we have all other's things expalained in this ISSUE?

@Danp2
Copy link
Owner

Danp2 commented Feb 2, 2022

Is this the only one thing which should be done as PR ?

I don't think so, as you've shown that this can be done with _WD_ElementOptionSelect.

Maybe you meant to show the retrieval of selectedIndex instead?

@mlipok
Copy link
Contributor Author

mlipok commented Feb 2, 2022

Maybe you meant to show the retrieval of selectedIndex instead?

Exactly.
Not for seting but for geting, it will be very similar to 'value' but value is a different things.

@mlipok
Copy link
Contributor Author

mlipok commented Feb 2, 2022

After your's thumbs up, the last things to confirm here.

Do we have all other's things expalained in this ISSUE?

If so this issue can be closed.

@Danp2
Copy link
Owner

Danp2 commented Feb 2, 2022

We discussed changes to $sSelector description and remarks. Did you plan to make those changes?

@mlipok
Copy link
Contributor Author

mlipok commented Feb 3, 2022

Yes.
I close this ISSUE as soon as I submit PR.
Then you will review in PR.
Ok ?

@mlipok
Copy link
Contributor Author

mlipok commented Feb 4, 2022

; Remarks .......: You can indicate specific element by Index, Value, Text by providing proper XPath or CSSSelector string as a $sSelector parameter

Again, I feel this is self-explanatory. It's simply how webdriver works. Maybe this would be better suited for a "getting started" section of the wiki? 🤷‍♂️

I would not use this Remarks because this following $sSelector description was used

; Name ..........: _WD_ElementOptionSelect
.....
.....
;                  $sSelector     - Indicates how the WebDriver should traverse through the HTML DOM to locate the desired element(s).  Should point to <option> in element of type '<select>'
.....
.....

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

No branches or pull requests

2 participants