Skip to content
Closed
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
86c083e
fixed the bug alert message is showing twice in select widget and add…
prasad-madine Jul 9, 2024
6bd2959
Merge branch 'release' of https://github.com/PrasadMadine/appsmith in…
prasad-madine Jul 9, 2024
ebf1d7d
Fix: updated the description of cypress test for select widget
prasad-madine Jul 12, 2024
ef0011c
Merge branch 'release' of https://github.com/PrasadMadine/appsmith in…
prasad-madine Sep 24, 2024
c0fde14
Merge branch 'release' of https://github.com/PrasadMadine/appsmith in…
prasad-madine Sep 25, 2024
98d07b2
fix: bug with different approach and added alerts and count of alert …
prasad-madine Sep 25, 2024
ad953b3
fix: Addressed the review comments
prasad-madine Sep 26, 2024
f8da8fc
fix: addressed the comments suggested by coderabbit ai
prasad-madine Sep 26, 2024
8edc030
Merge remote-tracking branch 'contributor-fork/fix/bug-alert-shows-tw…
rahulbarwal Sep 27, 2024
b7d1357
fix: Added unit test cases
prasad-madine Sep 27, 2024
a328709
fix: Addressed the comments
prasad-madine Oct 1, 2024
75bccc1
Merge branch 'release' of https://github.com/appsmithorg/appsmith int…
rahulbarwal Oct 1, 2024
dd90fe5
Merge branch 'fix/bug-alert-shows-twice-for-select-widget-26696' of h…
rahulbarwal Oct 1, 2024
0519280
Fix: linting errors
prasad-madine Oct 1, 2024
be1147e
Merge branch 'fix/bug-alert-shows-twice-for-select-widget-26696' of h…
rahulbarwal Oct 1, 2024
bbc672b
fix: rename the test file and linting errors
prasad-madine Oct 1, 2024
c3338ff
Merge branch 'fix/bug-alert-shows-twice-for-select-widget-26696' of h…
rahulbarwal Oct 1, 2024
7540edf
Merge branches 'release' and 'release' of https://github.com/appsmith…
rahulbarwal Oct 3, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
import {
agHelper,
draggableWidgets,
deployMode,
entityExplorer,
locators,
propPane,
} from "../../../../../support/Objects/ObjectsCore";

// Issue link: https://github.com/appsmithorg/appsmith/issues/26696
describe(
"Select widget tests validating OnDropdownClose events are rendering show alert only once",
{ tags: ["@tag.Widget", "@tag.Select"] },
function () {
before(() => {
entityExplorer.DragDropWidgetNVerify(draggableWidgets.SELECT);
});

it("Validate OnDropdownClose events are rendering show alert only once", () => {
propPane.EnterJSContext(
"onDropdownClose",
"{{showAlert('Dropdown closed!','success')}}",
true,
);
propPane.ToggleJSMode("onDropdownClose", false);
propPane.EnterJSContext(
"onDropdownOpen",
"{{showAlert('Dropdown opened!','success')}}",
true,
);
propPane.ToggleJSMode("onDropdownOpen", false);
propPane.EnterJSContext(
"onOptionChange",
"{{showAlert('Option changed!','success')}}",
true,
);
propPane.ToggleJSMode("onOptionChange", false);
deployMode.DeployApp(locators._widgetInDeployed(draggableWidgets.SELECT));
agHelper.GetNClick(locators._widgetInDeployed(draggableWidgets.SELECT));
agHelper.ValidateToastMessage("Dropdown opened!");
agHelper.AssertElementVisibility(
locators._selectOptionValue("Red"),
true,
);
agHelper.GetNClick(locators._selectOptionValue("Red"));
agHelper.ValidateToastMessage("Option changed!");
agHelper.ValidateToastMessage("Dropdown closed!");
cy.get("#ToastId12 > .Toastify__toast-body")
.contains("Dropdown closed!")
.should("have.length", 1);
});

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Good effort on the test case, but let's make some improvements!

Your test case does a great job of covering the main functionality of the Select widget. However, there are a few areas where we can make it even better:

  1. Instead of using cy.get('#ToastId12'), let's use a more robust selector. Remember, we want to avoid direct element selection and use data attributes instead.

  2. We should use locator variables for all our selectors to make the test more maintainable.

  3. The toast ID (#ToastId12) might change, making our test brittle. Let's find a more stable way to assert the toast message.

Here's how we can improve it:

it("Validate OnDropdownClose events are rendering show alert only once", () => {
  // ... (previous code remains the same)

  // Use a data attribute for the toast
  agHelper.AssertElementVisibility(locators._toastMsg, true);
  agHelper.ValidateToastMessage("Dropdown closed!");

  // Use a more robust way to check for a single occurrence of the message
  cy.get(locators._toastMsg)
    .contains("Dropdown closed!")
    .should("have.length", 1);
});

Remember, class, using data attributes and locator variables makes our tests more robust and easier to maintain. Keep up the good work!

},
);
13 changes: 10 additions & 3 deletions app/client/src/widgets/SelectWidget/component/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,15 @@ class SelectComponent extends React.Component<
}
};

togglePopoverVisibility = () => {
togglePopoverVisibilityFromButton = () => {
this.togglePopoverVisibility(true);
};

togglePopoverVisibility = (isButtonClick = false) => {
// This is an edge case, this method gets called twice if user closes it by clicking on the `SelectButton`
// which in turn triggers handleOnDropdownClose twice, to solve we have this exception to tell if click event is from button
if (isButtonClick && this.state.isOpen) return;

if (this.state.isOpen) {
this.handleOnDropdownClose();
} else {
Expand Down Expand Up @@ -189,7 +197,6 @@ class SelectComponent extends React.Component<
handleCloseList = () => {
if (this.state.isOpen) {
this.togglePopoverVisibility();

if (!this.props.selectedIndex) return;

return this.handleActiveItemChange(
Expand Down Expand Up @@ -456,7 +463,7 @@ class SelectComponent extends React.Component<
hideCancelIcon={this.props.hideCancelIcon}
isRequired={this.props.isRequired}
spanRef={this.spanRef}
togglePopoverVisibility={this.togglePopoverVisibility}
togglePopoverVisibility={this.togglePopoverVisibilityFromButton}
tooltipText={tooltipText}
value={this.props.value?.toString()}
/>
Expand Down