Skip to content

Support time inputs#245

Merged
route merged 2 commits intorubycdp:mainfrom
Antsiscool:support-time-inputs
Mar 28, 2026
Merged

Support time inputs#245
route merged 2 commits intorubycdp:mainfrom
Antsiscool:support-time-inputs

Conversation

@Antsiscool
Copy link
Copy Markdown
Contributor

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.

@Antsiscool
Copy link
Copy Markdown
Contributor Author

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

@brodienguyen
Copy link
Copy Markdown

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?

@Antsiscool Antsiscool force-pushed the support-time-inputs branch from 44bc276 to 85b71ef Compare January 4, 2024 05:26
@Antsiscool
Copy link
Copy Markdown
Contributor Author

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

@route
Copy link
Copy Markdown
Member

route commented Jan 5, 2024

Just approved them to run

@route
Copy link
Copy Markdown
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
Copy Markdown

@Slotos Slotos Mar 4, 2024

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.

@janwerkhoven
Copy link
Copy Markdown

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
@Antsiscool
Copy link
Copy Markdown
Contributor 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.

@Antsiscool
Copy link
Copy Markdown
Contributor 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

@route
Copy link
Copy Markdown
Member

route commented Mar 28, 2026

Rebasing and running tests locally. If they all pass, I'll merge manually

@route route merged commit f46cd3f into rubycdp:main Mar 28, 2026
4 of 6 checks passed
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

Successfully merging this pull request may close these issues.

5 participants