Make it possible to lock modal completely

[houmark]

Feature description

This may be possible already, but I did not find a way, so here we go.

I'd like to be able to lock a modal from being closed by clicking on the modal backdrop (and potentially even clicking or removing the X in such cases).

There are use cases where the user is doing very important work in a modal or you want them to fully acknowledge a change before they can move on, and for those cases, it feels a bit dangerous that they can just click the backdrop and have it close. I know, in a perfect world, you'd keep the state and have everything as they left it when they re-open the modal, but I still think a way to lock it up (even programmatically after opening it) would be nice. Clicking the right button would then unlock and close or whatever I'd like.

I'm still not fluent enough in TypeScript to contribute myself, but I see there's some vscode stuff coming in which may help me get started, but I wanted to vet the interest in this feature before doing more.

Or maybe it's already possible in some way I did not discover?

[chasestarr]

Hmm I assumed that the closeable prop on Modal would do what you're looking for. https://codesandbox.io/s/modal-7khwy

This looks like a regression that needs to be fixed

[houmark]

Yes, closable should do that but currently has no effect.

[gergelyke]

I am working on this one

[gergelyke]

@chasestarr @houmark referenced a PR above with the "fix" - it seems like none of us noticed the tiny typo in the codesandbox :)

[houmark]

Reposting my PR comment here with additional thoughts:

That's so weird. I'm sure I tried this before and it didn't work. But I just tested and it works as intended with the right prop :)

Now that we are around, maybe deprecating this prop name and introduce either isCloseable or better dismissOnClickOutside (which is the prop name used for Popover) is a good idea to have more consistent naming?

After a second look at this, closeable will also remove the X in the upper right corner of the modal, which is not exactly my ideal expectation when using that prop. I would still like to have that close button/icon, so the user can click that fairly small X or a cancel button (if I include such a button) in the bottom of the modal. So with that in mind, the current prop should probably stay as is and then introduce dismissOnClickOutside which will keep the X but prevent closing on click on the backdrop? Best of both worlds and either can be used depending on the use case.

The one use case that I don't think is "solved" by doing both is accidental escape hits. If you usecloseable then you have no X, and escape is effectively disabled, but if you only use dismissOnClickOutside, it would probably still close on escape — unless escape is also disabled for that prop which would be my preferred personal option. A third route is to introduce a prop for closing with escape, which makes the developer able to mix match in total depending on the use case and using closeable is combining some of them.

I hope it makes sense. If not, I'd be happy to elaborate.

What do you think?

[houmark]

And if this is something you'd accept, then the Drawer component should get all the same props in the same round, as it works and behaves almost the same as Modal, and was based on Modal.

[chasestarr]

reopening because closing pr solved initial issue, but some improvements can be made. description here: https://github.com/uber/baseweb/pull/2686#pullrequestreview-347554259

[houmark]

Thanks for re-opening. I take from your PR comment, that you are open to both my suggestions.

If that's the case, I may, unless @foodforarabbit beats me to it, attempt my first PR for these features as I think they are a perfect first case for someone with little TS experience :)

[foodforarabbit]

Hehe go ahead feel free to ask if you need help! This code-base is clean and nicely organized easy to learn on :D

[houmark]

Honestly, my main issue is the initial setup, how to get everything going, including a setup where I can see changes done either in our UI (possibly with npm link) or in the playground. My only TS knowledge is reading various libraries that is based on TS including Baseweb. Our platform is JavaScript all around.

I saw recently some commits related to vscode which is my editor of choice, but not sure if that's related to this or not.

[foodforarabbit]

clone the repo, check out master, pull to make sure you have the latest.

install nvm if you haven't already type nvm use (it will switch to the correct version of node, or prompt you to install it) (I don't know if yarn comes preinstalled but you might have to install that too, just do npm install -g yarn then, similar to typical webpack app yarn -> install dependencies yarn documentation:dev:watch -> leave this running somewhere and start up localhost:3000. I do most of my dev work on the component page itself / examples etc...

yarn e2e:serve (in one window) yarn e2e:build && yarn e2e:test -> e2e tests yarn test -> linting, flow, typescript and unit tests

most components are in their own folder (e.g. src/popover/index.js) and just find the various pieces from there tests are under there in tests there are unit tests (jest) in the *.test.js files puppeteer 'scenarios' (e.g. pages) are in *.scenario.js e2e tests are in *.e2e.js - check one of them out. Basic usage they await a page, which is the name export in the *.scenario.js file. Then you use selectors to clicking things and check things

the documentation page itself is generated with <component>.mdx e.g. documentation-site/pages/components/popover.mdx not sure exactly how it works I just copied the existing examples

the big test box on top of the documentation page is the "yard" and you can change it in the config e.g. documentation-site/components/yard/config/popover.ts

Yea I use vscode too, there is one plugin I have that is VERY useful... some eslint plugin that auto fixes on save. (it's called ESLint with 7.5 million downloads). You have to configure it to autofix on save, google it I can't remember how.

Gitlens is also very useful, oftentimes if you see some lines of code that you're curious about it is helpful to jump to the commit to see it in context / comments.

If you really need to, I think you can yarn link in the baseui repo and then yarn link baseui in the repo you want to run your latest code on, but I find that messing around with examples and testing on the development site is easier.

That's all I can think of for now, the rest is up to you! good luck!

On Thu, Jan 23, 2020 at 3:57 PM houmark [email protected] wrote:

Honestly, my main issue is the initial setup, how to get everything going, including a setup where I can see changes done either in our UI (possibly with npm link) or in the playground. My only TS knowledge is reading various libraries that is based on TS including Baseweb. Our platform is JavaScript all around.

I saw recently some commits related to vscode which is my editor of choice, but not sure if that's related to this or not.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/uber/baseweb/issues/2673?email_source=notifications&email_token=AAR5OZ4L5DSY7QEK2TL6ABTQ7IVH7A5CNFSM4KKA6AI2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEJZJCQQ#issuecomment-577933634, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAR5OZ7BLJBUDH7OR35J7XTQ7IVH7ANCNFSM4KKA6AIQ .

[houmark]

Thanks, this is very helpful. I'm already using ESlint, and fixing on autosave. I'll give a shot soon!

[milkcask]

Let's assume we add 2 props: dismissOnClickOutside and dismissOnEsc.

We could simply add an OR in onEscape and onBackdropClick.

And I think closeable: false should override dismissOnClickOutside: true and dismissOnEsc: true for consistent behaviour. It should also give a warning like 'baseui:Modal Property "dismissOnClickOutside" must be false when "closeable" is false.'.

I'm going to give this a shot as my first issue here.