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

accept options.ensure to disable actionability checks #4881

Merged
merged 8 commits into from
Sep 24, 2019

Conversation

kuceb
Copy link
Contributor

@kuceb kuceb commented Jul 30, 2019

fix #4874

tasks

  • rename ensureReceivability -> ensureNotDisabled

Pre-merge Tasks

  • Have tests been added/updated for the changes in this PR?
  • Has the original issue been tagged with a release in ZenHub?

@brian-mann
Copy link
Member

What do you think of calling this option validate vs ensure ?

@jennifer-shehane
Copy link
Member

I like ensure better, since validate has it's own very specific meaning in the DOM - ensure causes less confusion that it's actually associated with DOM validation in some way.

@kuceb kuceb requested review from jennifer-shehane and brian-mann and removed request for jennifer-shehane July 31, 2019 16:35
@kuceb
Copy link
Contributor Author

kuceb commented Jul 31, 2019

another question is whether we want this exposed in our command options API, similar to how users can do waitForAnimations.

If we do want to expose this in options API, we should probably do so in a similar verbiage to waitForAnimations, since that is configuring very similar behavior

@cypress
Copy link

cypress bot commented Jul 31, 2019



Test summary

3341 0 47 0


Run details

Project cypress
Status Passed
Commit 68f065c
Started Aug 30, 2019 7:01 PM
Ended Aug 30, 2019 7:05 PM
Duration 04:12 💡
OS Linux Debian - 8.10
Browser Multiple

View run in Cypress Dashboard ➡️


This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

@kuceb
Copy link
Contributor Author

kuceb commented Jul 31, 2019

I think this should go in. This does not affect end-user API and only changes one command file

Copy link
Member

@jennifer-shehane jennifer-shehane left a comment

Choose a reason for hiding this comment

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

Have some notes on possibly changing the names of a couple methods, logic looks good though. 👍

packages/driver/src/cy/actionability.coffee Outdated Show resolved Hide resolved
packages/driver/src/dom/elements.js Show resolved Hide resolved
@kuceb kuceb requested a review from jennifer-shehane August 30, 2019 18:39
@jennifer-shehane jennifer-shehane merged commit f32a921 into develop Sep 24, 2019
@@ -153,6 +153,14 @@ describe "src/cy/commands/actions/focus", ->
cy.get("[data-cy=rect]").focus().then ->
expect(onFocus).to.be.calledOnce

it "can focus on readonly inputs", ->
Copy link
Member

Choose a reason for hiding this comment

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

Because this is the test that is effectively testing this entire PR - it needs a link above the issue to the issue that it's fixing.

grabartley pushed a commit to grabartley/cypress that referenced this pull request Oct 6, 2019
)

* accept ensure option on actionability

* remove left-in code

* update type_spec for changed error message

* remove unneeded code, allow focused to be validated for readonly

* address TODO in type.coffee about scrolling before typing into already focused

* rename ensureReceiveablility -> ensureNotDisabled, revert error message
@emilyrohrbough emilyrohrbough deleted the readonly-actionability branch August 1, 2024 13:46
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.

Cannot click on 'readonly' inputs / textarea
3 participants