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

[BUG] EuiComboBox incorrectly enters invalid state #8

Closed
AvivBenchorin opened this issue Jul 20, 2021 · 1 comment · Fixed by #14
Closed

[BUG] EuiComboBox incorrectly enters invalid state #8

AvivBenchorin opened this issue Jul 20, 2021 · 1 comment · Fixed by #14
Assignees
Labels
bug Something isn't working

Comments

@AvivBenchorin
Copy link
Owner

Describe the bug

When searching for a specific option in a EuiComboBox field and clicking on the desired option, the element sometimes enters an invalid state rather than select the option. The invalid state is visually indicated through a red line along the bottom of the input field.

This bug happens inconsistently, and usually results in the test case failing due to preventing Cypress from performing subsequent interactions with UI elements (e.g. Cypress being unable to press the submit button since the option was not properly selected). It is likely caused by a race condition between the combo box re-rendering after the selection of the option and the combo box checking whether it is in the invalid state.

To Reproduce

This bug is difficult to reproduce at-will due to it occurring inconsistently. The current best method would be:

  1. (Optional) add .only() to the 'Explore the data in the dashboard' test suite in the demo_spec.js file (e.g. it.only('Explore the data in the dashboard',... )
  2. Open the Cypress Test Runner (e.g. npx cypress open) and run the demo_spec.js test
  3. Rerun the test until the error occurs, most frequently when selecting Gnomehouse in the Manufacturer field or when selecting day_of_week in the Add filter field input

Expected behavior

Cypress should consistently be able to type into the input field of the combo box, find the desired option, and click on the option to select it without the combo box entering an invalid state.

OpenSearch Version

1.0.0-SNAPSHOT

Dashboards Version

1.0.0

Additional Context

The EuiComboBox is a option list UI element that allows users to search for options and select multiple options if wanted. The OpenSearch Dashboards interface uses combo boxes to handle functionality that potentially requires searching a large number of options, such as in the Add filter panel.

When developing OpenSearch Dashboards functional tests in Cypress, the standard approach to select an option in a combo box has been the following series of actions:

  1. Open the combo box if it is not already visible (e.g. click the add Filter button)
  2. Click on the input field and type the keyword of interest (e.g. click on the Field input and type day_of_week)
  3. Ensure that the list has properly updated by asserting that the number of visible options is equal to the expected amount (e.g. assert that there are two options remaining after searching for day_of_week)
  4. Click on the desired option (e.g. click on the day_of_week option)

The bug of entering the invalid state has been occurring in step 4, after clicking the button. The EuiComboBox source code contains the following comments and code about the invalid state:

// Visually indicate the combobox is in an invalid state if it has lost focus but there is text entered in the input.
// When custom options are disabled and the user leaves the combo box after entering text that does not match any
// options, this tells the user that they've entered invalid input.
const markAsInvalid =
      isInvalid ||
      ((hasFocus === false || isListOpen === false) && searchValue);

The above indicates that the invalid state is being caused by the list being closed or losing focus while there is still a searchValue in the input. This suggests that in the context of the bug, there is likely a race condition created when the option is clicked and the option list closes:

  • In the first scenario, the combo box updates with the option selection first, clearing the searchValue and preventing the invalid state from being true.
  • In the second scenario, the combo box checks the invalid state first and finds that the list is closed and the searchValue is filled, concluding that the invalid state is true.

Assuming that this evaluation of the bug's cause is correct, the fix will likely involve addressing this race condition and ensuring that the option selection completes before checking the invalid state.

AvivBenchorin added a commit that referenced this issue Jul 20, 2021
Adding new dashboard test suite to the demo functional test, as well as updating the method of verifying the OpenSearch Dashboards home page has loaded properly.

Adds known flakiness through the testing of the EuiComboBox element, which introduces inconsistent behavior with the ComboBox “isinvalid” state when trying to click on an option after entering text in the search box. This bug is further discussed in #8.

Signed-off-by: Aviv Benchorin <benchori@amazon.com>
AvivBenchorin added a commit that referenced this issue Jul 20, 2021
Adding new dashboard test suite to the demo functional test, as well as updating the method of verifying the OpenSearch Dashboards home page has loaded properly.

Adds known flakiness through the testing of the EuiComboBox element, which introduces inconsistent behavior with the ComboBox “isinvalid” state when trying to click on an option after entering text in the search box. This bug is further discussed in #8.

Signed-off-by: Aviv Benchorin <benchori@amazon.com>
@AvivBenchorin AvivBenchorin self-assigned this Jul 20, 2021
@AvivBenchorin AvivBenchorin added the bug Something isn't working label Jul 21, 2021
AvivBenchorin added a commit that referenced this issue Jul 21, 2021
Adds a new Cypress test file dashboard_filtering_spec.js, which tests whether dashboard visualizations properly update when applying and disabling filters. This functional test is a Cypress adaptation of the dashboard_filtering.js test file from OpenSearch Dashboards, which originally used the Selenium framework. The mapping and index archives used in the OpenSearch Dashboards test have been added to the fixtures directory.

The test file is incomplete, and lacks the functionality to import the indices and data used in the test to OpenSearch. The test also contains known inconsistent behavior with combo boxes (described in #8).

Signed-off-by: Aviv Benchorin <benchori@amazon.com>
AvivBenchorin added a commit that referenced this issue Jul 21, 2021
Adds a new Cypress test file dashboard_filtering_spec.js, which tests whether dashboard visualizations properly update when applying and disabling filters. This functional test is a Cypress adaptation of the dashboard_filtering.js test file from OpenSearch Dashboards, which originally used the Selenium framework. The mapping and index archives used in the OpenSearch Dashboards test have been added to the fixtures directory.

The test file is incomplete, and lacks the functionality to import the indices and data used in the test to OpenSearch. The test also contains known inconsistent behavior with combo boxes (described in #8).

Signed-off-by: Aviv Benchorin <benchori@amazon.com>
AvivBenchorin added a commit that referenced this issue Jul 26, 2021
Adds a new Cypress test file dashboard_filtering_spec.js, which tests whether dashboard visualizations properly update when applying and disabling filters. This functional test is a Cypress adaptation of the dashboard_filtering.js test file from OpenSearch Dashboards, which originally used the Selenium framework. The mapping and index archives used in the OpenSearch Dashboards test have been added to the fixtures directory.

The test file is incomplete, and lacks the functionality to import the indices and data used in the test to OpenSearch. The test also contains known inconsistent behavior with combo boxes (described in #8).

Signed-off-by: Aviv Benchorin <benchori@amazon.com>
AvivBenchorin added a commit that referenced this issue Jul 28, 2021
Adds a new Cypress test file dashboard_filtering_spec.js, which tests whether dashboard visualizations properly update when applying and disabling filters. This functional test is a Cypress adaptation of the dashboard_filtering.js test file from OpenSearch Dashboards, which originally used the Selenium framework. The mapping and index archives used in the OpenSearch Dashboards test have been added to the fixtures directory.

The test file is incomplete, and lacks the functionality to import the indices and data used in the test to OpenSearch. The test also contains known inconsistent behavior with combo boxes (described in #8).

Signed-off-by: Aviv Benchorin <benchori@amazon.com>
AvivBenchorin added a commit that referenced this issue Jul 29, 2021
Adds a new Cypress test file dashboard_filtering_spec.js, which tests whether dashboard visualizations properly update when applying and disabling filters. This functional test is a Cypress adaptation of the dashboard_filtering.js test file from OpenSearch Dashboards, which originally used the Selenium framework. The mapping and index archives used in the OpenSearch Dashboards test have been added to the fixtures directory.

The test file is incomplete, and lacks the functionality to import the indices and data used in the test to OpenSearch. The test also contains known inconsistent behavior with combo boxes (described in #8).

Signed-off-by: Aviv Benchorin <benchori@amazon.com>
AvivBenchorin added a commit that referenced this issue Jul 29, 2021
Adds a new Cypress test file dashboard_filtering_spec.js, which tests whether dashboard visualizations properly update when applying and disabling filters. This functional test is a Cypress adaptation of the dashboard_filtering.js test file from OpenSearch Dashboards, which originally used the Selenium framework. The mapping and index archives used in the OpenSearch Dashboards test have been added to the fixtures directory.

The test file is incomplete, and lacks the functionality to import the indices and data used in the test to OpenSearch. The test also contains known inconsistent behavior with combo boxes (described in #8).

Signed-off-by: Aviv Benchorin <benchori@amazon.com>
AvivBenchorin added a commit that referenced this issue Jul 29, 2021
Adds a new Cypress test file dashboard_filtering_spec.js, which tests whether dashboard visualizations properly update when applying and disabling filters. This functional test is a Cypress adaptation of the dashboard_filtering.js test file from OpenSearch Dashboards, which originally used the Selenium framework. The mapping and index archives used in the OpenSearch Dashboards test have been added to the fixtures directory.

The test file is incomplete, and lacks the functionality to import the indices and data used in the test to OpenSearch. The test also contains known inconsistent behavior with combo boxes that is minimized through the use of a helper function that adds dashboard filters (described in #8).

This commit also adds custom Cypress commands that perform common UI interactions, such as setting a filter in a dashboard or checking for the exi
stence of a specified UI element. These custom Cypress commands should help with the readability of test files, and well as make it easier to develop new functional tests.

Signed-off-by: Aviv Benchorin <benchori@amazon.com>
AvivBenchorin added a commit that referenced this issue Jul 29, 2021
Adds a new Cypress test file dashboard_filtering_spec.js, which tests whether dashboard visualizations properly update when applying and disabling filters. This functional test is a Cypress adaptation of the dashboard_filtering.js test file from OpenSearch Dashboards, which originally used the Selenium framework. The mapping and index archives used in the OpenSearch Dashboards test have been added to the fixtures directory.

The test file is incomplete, and lacks the functionality to import the indices and data used in the test to OpenSearch. The test also contains known inconsistent behavior with combo boxes that is minimized through the use of a helper function that adds dashboard filters (described in #8).

This commit also adds custom Cypress commands that perform common UI interactions, such as setting a filter in a dashboard or checking for the exi
stence of a specified UI element. These custom Cypress commands should help with the readability of test files, and well as make it easier to develop new functional tests.

Signed-off-by: Aviv Benchorin <benchori@amazon.com>
@AvivBenchorin
Copy link
Owner Author

Issue Update

I have found a temporary solution that prevents EuiComboBox fields from incorrectly entering an invalid state when selecting an option, which has helped reduce inconsistency when interacting with the Add filter combo boxes that are present throughout OpenSearch Dashboards. The following helper function is able to get a given combo box input field, type a keyword into it, and then click on the desired option:

  const selectComboBoxInput = (selector, keyword) => {
    cy.get('[data-test-subj="' + selector + '"]').find('[data-test-subj="comboBoxInput"]').trigger('focus').type('{selectall}{backspace}' + keyword)
    cy.get('[data-test-subj="comboBoxOptionsList ' + selector + '-optionsList"]').find('[title="' + keyword + '"]').trigger('click', { force: true })
  }

The function above is being added in #14, and is being used inside of a larger helper function that handles adding a dashboard filter. Since starting to use this approach for selecting combo box options, I have not yet encountered the invalid state issue, although this is only empirical evidence for the inconsistency being resolved.

Explanation

  cy.get('[data-test-subj="filterFieldSuggestionList"]').find('[data-test-subj="comboBoxInput"]').type(field)
  cy.get('[data-test-subj="comboBoxOptionsList filterFieldSuggestionList-optionsList"]').find('[title="' + field + '"]').click({ force: true })

The above code is one of my previous attempts to select an option from a combo box list, which have generally utilized the Cypress .click() function. This function under the hood performs the default browser actions for performing the desired interaction; for example the following image shows the mouse events that occurred during a "successful" .click() operation that selected a combo box option:

Pasted Graphic 2

And the following image shows the mouse events during a .click() operation that caused the combo box to enter the invalid state:

Stopped Propagation Active Modifiers

Based on the error message in the mouseup event, the only differences between the events of the two clicks were that the mouseup event didn't fire and that the pointerup event was cancelled. My hypothesis is that while Cypress is performing these mouse events leading up to the click itself, there exists a race condition between the click action being performed, and the combo box input re-rendering and finding itself to be in an invalid state (cancelling the pointerup event in the process).

Given this hypothesis, a possible solution to preventing the inconsistent invalid state was to eliminate the race condition altogether by not performing the default mouse actions leading up to the click. This was implemented by using the Cypress .trigger('click') operation rather than .click() (the difference between the two is discussed here). As well, the Cypress .type() command issues a click event on the chosen input box if it had not already been focused on (read more here), which caused the same type of race condition to occur. To remedy this, I called .trigger('focus') before the .type() command to prevent the unnecessary click from being performed.

AvivBenchorin added a commit that referenced this issue Jul 30, 2021
Adds a new Cypress test file dashboard_filtering_spec.js, which tests whether dashboard visualizations properly update when applying and disabling filters. This functional test is a Cypress adaptation of the dashboard_filtering.js test file from OpenSearch Dashboards, which originally used the Selenium framework. The mapping and index archives used in the OpenSearch Dashboards test have been added to the fixtures directory.

The test file is incomplete, and lacks the functionality to import the indices and data used in the test to OpenSearch. The test also contains known inconsistent behavior with combo boxes that is minimized through the use of a helper function that adds dashboard filters (described in #8).

This commit also adds custom Cypress commands that perform common UI interactions, such as setting a filter in a dashboard or checking for the exi
stence of a specified UI element. These custom Cypress commands should help with the readability of test files, and well as make it easier to develop new functional tests.

Signed-off-by: Aviv Benchorin <benchori@amazon.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant