Skip to content
Snippets Groups Projects
Unverified Commit bc92d8ca authored by Howon Lee's avatar Howon Lee Committed by GitHub
Browse files

Check to see if `done` button is enabled whenever custom expr changes (#15293)

This is done by firing an event on change which mutates if blankness of the input changes
parent 922b9036
Branches
Tags
No related merge requests found
......@@ -12,6 +12,7 @@ import "./ExpressionPopover.css";
export default class ExpressionPopover extends React.Component {
state = {
error: null,
isBlank: true,
};
render() {
......@@ -26,11 +27,11 @@ export default class ExpressionPopover extends React.Component {
name,
onChangeName,
} = this.props;
const { error } = this.state;
const { error, isBlank } = this.state;
// if onChangeName is provided then a name is required
const isValid = !error && (!onChangeName || name);
const buttonEnabled = !error && !isBlank && (!onChangeName || name);
// if onChangeName is provided then a name is required
return (
<div className="ExpressionPopover">
<div className="text-medium p1 py2 border-bottom flex align-center">
......@@ -56,6 +57,9 @@ export default class ExpressionPopover extends React.Component {
onDone(expression);
}
}}
onBlankChange={newBlank => {
this.setState({ isBlank: newBlank });
}}
/>
{onChangeName && (
<input
......@@ -63,7 +67,7 @@ export default class ExpressionPopover extends React.Component {
value={name}
onChange={e => onChangeName(e.target.value)}
onKeyPress={e => {
if (e.key === "Enter" && isValid) {
if (e.key === "Enter" && buttonEnabled) {
onDone(expression);
}
}}
......@@ -73,7 +77,7 @@ export default class ExpressionPopover extends React.Component {
<Button
className="full"
primary
disabled={!isValid}
disabled={!buttonEnabled}
onClick={() => onDone(expression)}
>
{t`Done`}
......
......@@ -109,6 +109,7 @@ export default class ExpressionEditorTextfield extends React.Component {
onChange: PropTypes.func.isRequired,
onError: PropTypes.func.isRequired,
startRule: PropTypes.string.isRequired,
onBlankChange: PropTypes.func,
};
static defaultProps = {
......@@ -300,6 +301,9 @@ export default class ExpressionEditorTextfield extends React.Component {
};
const isValid = expression !== undefined;
if (this.props.onBlankChange) {
this.props.onBlankChange(source.length === 0);
}
// don't show suggestions if
// * there's a selection
// * we're at the end of a valid expression, unless the user has typed another space
......
......@@ -526,7 +526,7 @@ describe("scenarios > question > filter", () => {
cy.findByText(AGGREGATED_FILTER);
});
it("in a simple question should display popup for custom expression options (metabase#14341)", () => {
it("in a simple question should display popup for custom expression options (metabase#14341) (metabase#15244)", () => {
openProductsTable();
cy.findByText("Filter").click();
cy.findByText("Custom Expression").click();
......@@ -543,6 +543,7 @@ describe("scenarios > question > filter", () => {
.click()
.type("contains(");
cy.findByText(/Checks to see if string1 contains string2 within it./i);
cy.findByRole("button", { name: "Done" }).should("not.be.disabled");
cy.get(".text-error").should("not.exist");
cy.findAllByText(/Expected one of these possible Token sequences:/i).should(
"not.exist",
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment