Request: containerId prop

[johncip]

Hey, I like your modal component and have been using it. Unlike many others, it doesn't have a ton of dependencies, or do nasty things with focus, or add 50k to my page weight :)

One thing that would be useful is if the API allowed setting the ID of the "container" div, in addition to its class. Using IDs helps with CSS specificity, and often you only have one instance of a modal anyway, which using IDs helps to document in the stylesheet.

(I know I have full control over the child elements I pass in, but I'm currently using your modal in a context where adding another div isn't a great option.)

On that note, I think it'd also be nice if instead of className and containerClassName, the props were overlayClassName and className, respectively. Then using it would look something like,

<Modal id='apple-modal' className='responsive-modal'>
  Apples
</Modal>

<Modal id='banana-modal' className='responsive-modal'>
  Bananas
</Modal>

.responsive-modal
  max-width: 500px
  @media (max-width: 540px)
    width: calc(100% - 40px)

#apple-modal
  color: red

#banana-modal
  color: yellow

In my mind, "modal" is short for "modal dialog" and what you refer to as the container, I think of as the top-level element of that dialog.

[zackify]

Thanks for the suggestions. I never thought that many people would use this
honestly, but I am glad you like it. I will add the prop for you, and
rename to overlay. Also, I will try to add some accessibility things for
screen readers. Give me a day or two!
On Fri, Apr 29, 2016 at 00:46 John Cipriano notifications@github.com
wrote:

Hey, I like your modal component and have been using it. Unlike many
others, it doesn't have a ton of dependencies, or do nasty things with
focus, or add 50k to my page weight :)

One thing that would be useful is if the API allowed setting the ID of the
"container" div, in addition to its class. Using IDs helps with CSS
specificity, and often you only have one instance of a modal anyway, which
using IDs helps to document in the stylesheet.

(I know I have full control over the child elements I pass in, but I'm
currently using your modal in a context where adding another div isn't a
great option.)

On that note, I think it'd also be nice if instead of className and
containerClassName, the props were overlayClassName and className,
respectively. Then using it would look something like,

Apples Bananas

.responsive-modal
max-width: 500px
@media (max-width: 540px)
width: calc(100% - 40px)
#apple-modal
color: red
#banana-modal
color: yellow

In my mind, "modal" is short for "modal dialog" and what you refer to as
the container, I think of as the top-level element of that dialog.

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub
#13

[zackify]

Sorry, just kind of forgot about this. Are you still wanting this added?

[chrisstephens]

I would certainly like it to be added. As of today I am also using this code, also because it is just a simple scaffoldy thing. I'm trying to do a very particular kind of popup that nothing out-of-the-box will do and I was reluctant to start from scratch (deadlines, you know). Your code is pretty much exactly what I wanted to find - there must be others like me and johncip out there.

So thanks - and yes: please make the changes - both the ID and the renamed class props. :)

[johncip]

As it turns out, we're moving to SMACSS, and I most likely won't have to use an ID to get the right specificity. But it'd still be nice to have. There's no hurry on my end.