Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Support time inputs#245

Open
Antsiscool wants to merge 2 commits intorubycdp:main from
Antsiscool:support-time-inputs
Open

Support time inputs #245
Antsiscool wants to merge 2 commits intorubycdp:main from
Antsiscool:support-time-inputs

Conversation

@Antsiscool
Copy link

@Antsiscool Antsiscool commented Sep 26, 2023

I am converting a project over from capybara selenium and found that time inputs were failing to be correctly set.

This allows specifying the time as a string or a Time object. The date attributes will be ignored.

I have added test cases for both string and time. I also added some test cases for date inputs as well.

Copy link
Author

I am not sure why the tests are failing. They are all failing for me locally on the main branch as well.

Copy link

I am interested in following up this contribution as well.

@route Not sure if you are the project's maintainer, but could you give us some insights if you know why all the tests are falling please?

Copy link
Author

I have just rebased my changes onto the latest main branch.

Copy link
Member

route commented Jan 5, 2024

Just approved them to run

Copy link
Member

route commented Jan 7, 2024

Weird, dunno let me release new Ferrum and push my changes to cuprite and I'll get this into new release

Comment on lines 106 to 108
when "time"
value = value.strftime("%H:%M") if value.is_a?(Time)
node.evaluate("this.setAttribute('value', '#{value}')")
Copy link

@Slotos Slotos Mar 4, 2024
edited
Loading

Choose a reason for hiding this comment

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

I suggest changing lib/capybara/cuprite/javascripts/index.js to:

 } else if (node.type == "time") {
 this.setValue(node, value);
 this.input(node);
Suggested change
when "time"
value = value.strftime("%H:%M") if value.is_a?(Time)
node.evaluate("this.setAttribute('value', '#{value}')")
when "time"
value = value.strftime("%H:%M") if value.is_a?(Time)
command(:set,value.to_s)

This way, input event will be emitted correctly and value attribute won't be modified unnecessarily.

Copy link

Need this. +1 👀

Do not modify time input value in JS
Extract time inputs to seperate test file
The test inputs load a data attribute for an SVG icon. This is changing the count of network requests in unrelated tests.
Add support for week and month inputs
Copy link
Author

@route I have revisited this PR and cleaned it up. The issue with the original failing tests was that loading SVG icons for time and date inputs and this was increasing the count of network requests.

I have moved the time and date tests to a new HTML file and this has resolved the problem.

I have also added support for month and week inputs.

It seems the test suite is failing on Ruby 3.0 and Ruby 3.2. I have been unable to replicate these failures locally.

Copy link
Author

These failing tests seem to be flaky. I can replicate the issue locally.

I created a PR onto my fork to run the tests and they all passed. Antsiscool#1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@Slotos Slotos Slotos left review comments

Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /