Skip to content

Instantly share code, notes, and snippets.

@kvz
Last active February 21, 2016 15:16
  • Star 0 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
Star You must be signed in to star a gist
Save kvz/e9f5b113f07763e49161 to your computer and use it in GitHub Desktop.
todays-modal-todos.md
Some todos I can think off:
- [ ] Make Modal prettier and accessible using Artur's research
- [ ] Convert `GoogleDrive` to adhere to `Dummy`'s format, so it'sare compatible with the new Modal
- [ ] Convert `DragDrop` to adhere to `Dummy`'s format, so it'sare compatible with the new Modal
- [ ] Make `ProgressBar` work with the new Modal
- [ ] Rename FakeModal to Modal, deprecating our old one
- [ ] Make the Modal look like Harry's sketchup
- [ ] ?
@arturi
Copy link

arturi commented Feb 21, 2016

Some thoughts:

  1. Make sure modal renders under one dom node?
  2. Think about whether it’s better to use :after on body or a separate div for dimming effect? I guess it’s better not to touch body for React future reasons?
  3. Should Modal be aware of progressbars and spinners, is it a separate method, not prepareTarget?

@kvz
Copy link
Author

kvz commented Feb 21, 2016

  1. Make sure modal renders under one dom node?

  1. Think about whether it’s better to use :after on body or a separate div for dimming effect? I guess it’s better not to touch body for React future reasons?

makes sense (not touching body) (but saying this with little confidence as I'm a bit out my comfort zone)

  1. Should Modal be aware of progressbars and spinners, is it a separate method, not prepareTarget?

The way I thought of it, we'd have if (caller.type === 'acquire') {this} if (caller.type === 'progress') {that} in the prepareTarget

@hedgerh
Copy link

hedgerh commented Feb 21, 2016

Think about whether it’s better to use :after on body or a separate div for dimming effect? I guess it’s better not to touch body for React future reasons?

I haven't heard about using :after on the body as the approach for an overlay dimming effect, but I think that would be problematic since the modal lives within the body tag as well. It would dim over every single thing within the body. I think the approach uploadcare uses is to put the modal within the overlay element itself, so:

<div class="Uppy-overlay">
  <div class="Uppy-modal">
     ....
  </div>
</div> 

Then that could all be within a React component with a shouldComponentUpdate() that always returns false, I'd say?

@arturi
Copy link

arturi commented Feb 21, 2016

The way I thought of it, we'd have if (caller.type === 'acquire') {this} if (caller.type === 'progress') {that} in the prepareTarget
👍

modal within the overlay element itself, Then that could all be within a React component with a shouldComponentUpdate() that always returns false
👍

Great.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment