search

Danp2 / au3WebDriver

Web Driver UDF for AutoIt
MIT License
107 stars 23 forks source link

DemoAlerts() - show that prompt was set #371

Closed mlipok closed 2 years ago

mlipok commented 2 years ago

Pull request

Proposed changes

this PR is only related to the comment that describe in the specific case what kind of expected result should be to check Alert status

Checklist

Types of changes

What is the current behavior?

; check status before displaying Alert

What is the new behavior?

; check status before displaying Alert - this should set @error as there is no any Alert displayed yet

Additional context

I was checking examples for this ISSUE #254

System under test

not related

mlipok commented 2 years ago

I have some further refactoring for this DemoAlerts(). WIP

mlipok commented 2 years ago

As for me it is all in this PR, please review again and merge if you agree with my opinion about the comments:

The main point of the status option is to return either True or False. Wouldn't it be better to explain that?

If we change error handling then the comments change in this regards is not needed as there is already status reported to console

$sStatus = _WD_Alert($sSession, 'status')
ConsoleWrite("wd_demo.au3: (" & @ScriptLineNumber & ") : " & 'Alert Detected => ' & $sStatus & @CRLF)
Danp2 commented 2 years ago

using Storage to show that prompt was set

Why is this needed here? I don't see the purpose of using "storage" in the middle of an Alerts demo.

mlipok commented 2 years ago

To show that the given text entered to prompt was properly send to browser.

Danp2 commented 2 years ago

Sorry, but I don't understand how this code demonstrates that. With this line --

_WD_ExecuteScript($sSession, "window.localStorage.setItem('testing',prompt('User Prompt 1', 'Default value'))")

you introduce advanced concepts that have nothing to do with _WD_Alert IMO.

mlipok commented 2 years ago

Yes it is more advanced than only showing prompt. Because in order to check if ti was properly set by prompt it stores the value in webbrowser and then we check the stored value using _WD_Storage($sSession, 'testing')

Danp2 commented 2 years ago

I've run the revised demo, but I still don't get the point of this change. You are storing and retrieving the value that was changed with _WD_ExecuteScript, not _WD_Alert. Therefore, I don't see the added value here. 🤔

mlipok commented 2 years ago

this part:

https://github.com/Danp2/au3WebDriver/blob/a41396be2d1a71cd3d1015cafbb01c164124726e/wd_demo.au3#L531-L532 popup prompt to input data, which are saved in browser Storage

and this part: https://github.com/Danp2/au3WebDriver/blob/a41396be2d1a71cd3d1015cafbb01c164124726e/wd_demo.au3#L542-L544 check what trully was received and save in browser Storage

I call this validation.

EDIT: By the way, it also shows how Storage can be used.

Danp2 commented 2 years ago

I call this validation.

AFAICS you aren't validating the functionality of _WD_Alert. Therefore, this doesn't belong in DemoAlerts function.

By the way, it also shows how Storage can be used.

Also doesn't belong here.

mlipok commented 2 years ago

AFAICS you aren't validating the functionality of _WD_Alert. Therefore, this doesn't belong in DemoAlerts function.

So how we can validate that 

var userinput = prompt('User Prompt 1', 'Default value')

browser get the value provided by entering string to the prompt ?

Danp2 commented 2 years ago

You need a webpage that accepts the user input and then displays it on screen like they do here. I haven't tried this yet, but I was thinking that you could open a new tab and then use _WD_ExecuteScript to update the tab's source code to mimic the behavior of that website without the frames.

mlipok commented 2 years ago

Why not simply use this webpage ? https://www.quanzhanketang.com/jsref/tryjsref_prompt.html?filename=tryjsref_prompt