Ctrl+F to set focus on the search bar

[Manuel-Suarez-Abascal]

• Closes 6229

User facing changelog

There is a new keyboard shortcut (CTRL + F) to search spec files in the Test Runner.

Additional details

This PR allows users to perform a search for test files with the CTRL + F command. The file where the implementation was added is cypress\packages\desktop-gui\src\specs\specs-list.jsx. I used a method that uses the ternary operator & adds focus to the search input if CRTL + F is pressed. Then I called the method on keydown on the window object.

How has the user experience changed?

You can check the new behaviour in the gif below:

PR Tasks

• Has a PR for user-facing changes been opened in issue-6269?

[cypress-bot]

Thanks for the contribution! Below are some guidelines Cypress uses when doing PR reviews.

• Please write [WIP] in the title of your Pull Request if your PR is not ready for review - someone will review your PR as soon as the [WIP] is removed.
• Please familiarize yourself with the PR Review Checklist and feel free to make updates on your PR based on these guidelines.

PR Review Checklist

If any of the following requirements can't be met, leave a comment in the review selecting 'Request changes', otherwise 'Approve'.

User Experience

• The feature/bugfix is self-documenting from within the product.
• The change provides the end user with a way to fix their problem (no dead ends).

Functionality

• The code works and performs its intended function with the correct logic.
• Performance has been factored in (for example, the code cleans up after itself to not cause memory leaks).
• The code guards against edge cases and invalid input and has tests to cover it.

Maintainability

• The code is readable (too many nested 'if's are a bad sign).
• Names used for variables, methods, etc, clearly describe their function.
• The code is easy to understood and there are relevant comments explaining.
• New algorithms are documented in the code with link(s) to external docs (flowcharts, w3c, chrome, firefox).
• There are comments containing link(s) to the addressed issue (in tests and code).

Quality

• The change does not reimplement code.
• There's not a module from the ecosystem that should be used instead.
• There is no redundant or duplicate code.
• There are no irrelevant comments left in the code.
• Tests are testing the code’s intended functionality in the best way possible.

Internal

• The original issue has been tagged with a release in ZenHub.

[jennifer-shehane]

We will need some tests written to verify this new behavior.

To run the tests for the Desktop-GUI:

• within cypress root, run npm i
• cd to cypress/packages/desktop-gui and run npm run watch
• In cypress/packages/desktop-gui run npm run cypress:open
• click on the test you're writing to run it within Cypress

Instructions for running the desktop-gui tests can always be found here: https://github.com/cypress-io/cypress/blob/develop/packages/desktop-gui/README.md

The tests for this PR should be in this file specifically, involving searching the specs list: https://github.com/cypress-io/cypress/blob/develop/packages/desktop-gui/cypress/integration/specs_list_spec.js#L282:L282

[Manuel-Suarez-Abascal]

We will need some tests written to verify this new behavior.

@jennifer-shehane thanks for all the information. I have tried to add the following tests:

it('sets focus on search filters if user presses Ctrl + F', () => {
      cy.get('.filter').type('{ctrl}F')
      cy.get('.filter').should('to.be.focus')
})

It passes correctly, but when I tried to commit the changes, I get the following error message:

Found .only in folder(s) 👎
305:        it('displays only matching spec', () => {

Do I need to add this new test to a spec object or something else? Please advise on what to do to commit my changes 🙏 thanks in advance!

I also closed the branch by mistake 😱 , I have reopened it.. sorry about that...

[Manuel-Suarez-Abascal]

@jennifer-shehane I have tried adding the test in different places within the specs_list.spec.js file, but I still get the same error message when I tried to commit the changes.

I have attached a screenshot below showing that all tests passed properly:

[jennifer-shehane]

@Manuel-Suarez-Abascal The warning is indicating that you have a .only on this test file when you are trying to commit https://github.com/cypress-io/cypress/blob/develop/packages/desktop-gui/cypress/integration/specs_list_spec.js#L299:L299

But, if this is not the case, I think that there is an issue where this .only warning displays unnecessarily when trying to commit on Windows as documented here: #5518

Pass the --no-verify flag during your commit to ignore this warning.

[Manuel-Suarez-Abascal]

But, if this is not the case, I think that there is an issue where this .only warning displays unnecessarily when trying to commit on Windows as documented here: #5518

Pass the --no-verify flag during your commit to ignore this warning.

@jennifer-shehane thanks! It worked like a charm! I have pushed my latest changes.

[jennifer-shehane]

Shouldn't this be Meta + F? I instantly tried Meta + F (command on mac) because that is what inherently works in most browsers/applications to search for text and thought it wasn't working, but this is coded as Ctrl + f, which I don't think is a great experience.

This should match the behavior of other software's 'Find' keyboard shortcut.

Chrome:

VSCode:

[cypress]

---

Test summary

6863 • 0 • 97 • 0

---

Run details

Project	cypress
Status	Passed
Commit	dbefe71
Started	Feb 11, 2020 8:30 AM
Ended	Feb 11, 2020 8:36 AM
Duration	05:53 💡
OS	Linux Debian - 9.11
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

[Manuel-Suarez-Abascal]

@jennifer-shehane I see I thought we wanted to use/focus on the filter search when pressing Ctrl + F command.

I assume the GUI is built on Electron, is that accurated?

I'll have to find a way to get the BrowserWindow object on Electron because I don't think it has access to the browser's API. What are your thoughts on this?

[jennifer-shehane]

@Manuel-Suarez-Abascal Let me check with the team on the implementation we want and get back to you.

[Manuel-Suarez-Abascal]

@jennifer-shehane an update on this ticket.

After taking the time to think about the implementation, I decided to add an info icon & create click event to toggle the search input's placeholder from Search... to Press CTRL + F (or CMD + F on Mac) to make a quick search... & vice-versa.

In the beginning, I was thinking about on a mouseenter or mouseleave event, but I thought it was too intrusive & distracting.

Now, I need to add the functionality to the Electron menu. However, I wanted to know your opinion before moving forward in order to know if I'm on the right track.

I have attached a gif below. I think the info icon is...meh, so maybe you have another icon? 🤔

EDIT:

Any ideas on how to access the document from the electron menu file? So I could do something similar to this:

{
  label: 'Find..',
    click() {
    document.querySelector('#filter').focus()
  },
},

[jennifer-shehane]

• I would remove the info icon. I think that we should show the placeholder text containing the keyboard shortcuts on focus of the searchbar. This will make them aware of it in the future and not be distracting.
• We do not need to have text like 'if on Mac'. We know the operating system Cypress is running within, so you should show the appropriate command shortcut based off of the OS they are using.
• Yeah, I'm seeing now that Electron actually does not support a built in Find menu. I'm honestly not sure off the top of my head on how to get this shortcut to focus on the searchbar. It may need some kind of similar mechanism as onLogOutClicked uses. Feel free to reduce the scope of this PR to remove this requirement if needed.

[Manuel-Suarez-Abascal]

@jennifer-shehane

• I would remove the info icon. I think that we should show the placeholder text containing the keyboard shortcuts on focus of the searchbar. This will make them aware of it in the future and not be distracting.

Great idea! 💡 I have implemented it!

• We do not need to have text like 'if on Mac'. We know the operating system Cypress is running within, so you should show the appropriate command shortcut based off of the OS they are using.

I have a created method called isMac to check for the current Operating System & returns the desired message in the placeholder in consequence. I cannot test in different Operating System, so please let me know if it's working properly on different OS.

• Yeah, I'm seeing now that Electron actually does not support a built-in Find menu. I'm honestly not sure off the top of my head on how to get this shortcut to focus on the search bar. It may need some kind of similar mechanism as onLogOutClicked uses. Feel free to reduce the scope of this PR to remove this requirement if needed.

I'm not sure how to implement this behavior, so for now, I'll reduce the scope of this ticket.

[jennifer-shehane]

Testing this on a Mac, the display shows as Ctrl + F to make a quick search, but that seems contradictory to what your code says it should show.

Additionally the Ctrl + F keyboard shortcut is what functionally works on the Mac when I press it, but the keyboard shortcut we expect to use for Find on a Mac should be Cmd + F.

We do run automated tests in Windows and Linux, but....I don't think we're running these desktop-gui tests in Windows...but I would update the desktop-gui spec to account for which OS the test is actually running in since the behavior will be different when run on our devs Windows, Linux, or OS machines. You can check for this using Cypress.platform. https://on.cypress.io/platform

[Manuel-Suarez-Abascal]

You can check for this using Cypress.platform. https://on.cypress.io/platform

@jennifer-shehane I have tried to find the Operating System with this code snippet:

return (Cypress.platform === 'darwin')
      ? 'Press Cmd + F to make a quick search...'
      : 'Press Ctrl + F to make a quick search...'

But it states Cypress is not defined. I have tried importing it like so: import cypress from 'cypress', but it doesn't work.

I'm stuck on how to get the Operating System. In the beginning, I tried with os.platform(), but no luck.

Can you provide me with some pointers on how to accomplish this