MM-15128 - add support for interactive dialogs with no elements

[andresoro]

Summary

I think this simple change is what is needed to support interactive dialogs with no elements. Since my react/js knowledge is a little limited, I'm not sure if we need to check if the elements array is non null before we render or not.

Ticket Link

Part of MM-15128

Related Pull Requests

• Has server changes Pull request #10755

[hanzei]

Hi @andresoro,

Thanks for the PR. 👍

I've testing it with the demo plugin. See mattermost/mattermost-plugin-demo#28 for the PR that I used.

After creating the dialog with /dialog no-elements the web app crashes and I see the following error in the dev console:

Unhandled promise rejection TypeError: "props.elements is null"
    InteractiveDialog interactive_dialog.jsx:36

Let me know, if you need further information on how to debug this or have other questions.

[andresoro]

Hey @hanzei it looks like I need to add a check to see if the elements array is null or not. I suspected something like this would happen. I appreciate the update for the demo plugin and I am going to use that to test these changes further. Going to continue to work on this.

[andresoro]

I used the plugin mentioned above to test and debug the component. Checking for null in the constructor and the render is what's needed to display the interactive dialog properly. The "Confirm" button wasn't doing anything but I'm not sure if that's by design with the plugin or with the dialog component. Also, it seems like there is a blank space left over from Modal.Body since there are no elements.

It would look cleaner if the Modal.Body was dropped when the elements array is null.

[hanzei]

Thanks for the update! The dialog opens now!

The "Confirm" button wasn't doing anything but I'm not sure if that's by design with the plugin or with the dialog component.

The problem is caused by a TypeError:

Unhandled promise rejection TypeError: "elements is null"
    _callee$ interactive_dialog.jsx:56

Would you please fix this? Then we can queue this PR for PM review.

And could you also merge master into this? We did some changes to the build system yesterday. Sorry for the inconvenience.

[andresoro]

Went ahead and fixed that error and also changed the Modal.Body to display only if there are elements. I'm not sure if the way I did it was proper or hackish since my react/JS knowledge is rusty.

[hanzei]

@andresoro Thanks for the update. This works great! One visual problem I've noticed is that modal-header has a border-bottom and .modal-footer has a border-top. This makes up for a doubled border. Would you be open on changing this that it only shows one border?

[andresoro]

Two borders would be the expected behavior when we have elements to display, correct? So we should only have one border when there are no elements.

Kind of unsure how to go about this.

[hanzei]

@andresoro I see your points. Let's ask our Product Manager @jasonblais what he thinks about this.

[jasonblais]

Thanks for the contribution! Happy to take a look at it and see how it looks with two borders when there are no elements.

Looks like the build failed, can you help take a look at it @andresoro when you have the chance?

[andresoro]

Hey @jasonblais, I ran the commands that the CI ran and it works on my machine. I also did some searching and found that someone else had this same issue on mattermost-mobile #2405. Unsure how to proceed with this one.

Edit: as mentioned below, these were lint errors and I just hadn't pushed yet.

[cpanato]

some lint errors

/root/mattermost-webapp/components/interactive_dialog/interactive_dialog.jsx
   68:16  error  Trailing spaces not allowed  no-trailing-spaces
   70:1   error  Trailing spaces not allowed  no-trailing-spaces
  192:1   error  Trailing spaces not allowed  no-trailing-spaces

[jasonblais]

@cpanato The generated test server wasn't loading for me when I tried to access it. I've tried to generate a new one, but it seems to fail.

Next week, please help take a look what might be causing it?

[andresoro]

@jasonblais So I did a bit of research on this and found that React allows for inline styling within components. The docs say it is not the best way to style components but it also allows for conditional styling based on the components props. In our case it's whether or not there are elements in the dialog.

So I modified the Modal.Header tag to the following:

<Modal.Header
                    closeButton={true}
                    style={{borderBottom: elements == null && '0px'}}
>

Which essentially changes the border-bottom attribute to not show if there aren't elements. I tested with /dialog no-elements and the output looks like:

Edit: Update Modal.Header tag to not crash when regular dialogs are called.

[jasonblais]

Tested with the demo plugin Hanzei shared using

/dialog - Open an Interactive Dialog with a few elements
/dialog no-elements - Open an Interactive Dialog with no elements
/dialog help - Show this help text

Works for me as expected. Thanks @andresoro!

[hanzei]

@asaadmahmood @cpoile Gentle reminder to review this PR

[cpoile]

Nice, thank you! Just a little request, otherwise good to go.

[cpoile]

👍

[andresoro]

@cpoile thanks for the review. I'll make the changes and push 👍.

[cpoile]

Great, just remember to click request re-review so I know to come back. Thanks!

[cpoile]

nice work @andresoro!