Semantic-Org / Semantic-UI-React

The official Semantic-UI-React integration
https://react.semantic-ui.com
MIT License
13.21k stars 4.05k forks source link

Incorrect nested modal positions #4280

Open mariajosealvarez opened 3 years ago

mariajosealvarez commented 3 years ago

Bug Report

Steps

Create nested modals, adding a close icon on the second one:

You can use this snippet on modal examples:

import React from 'react'
import { Button, Icon, Modal, Image } from 'semantic-ui-react'

function ModalExampleMultiple() {
  const [firstOpen, setFirstOpen] = React.useState(false)
  const [secondOpen, setSecondOpen] = React.useState(false)

  return (
    <>
      <Button onClick={() => setFirstOpen(true)}>Open first Modal</Button>

      <Modal
        onClose={() => setFirstOpen(false)}
        onOpen={() => setFirstOpen(true)}
        open={firstOpen}
      >
        <Modal.Header>Modal #1</Modal.Header>
        <Modal.Content image>
          <div className='image'>
            <Icon name='right arrow' />
          </div>
         <Modal.Description>
          <p>
            This is an example of expanded content that will cause the modal's
            dimmer to scroll.
          </p>

          <Image
            src='https://react.semantic-ui.com/images/wireframe/paragraph.png'
            style={{ marginBottom: 10 }}
          />
          <Image
            src='https://react.semantic-ui.com/images/wireframe/paragraph.png'
            style={{ marginBottom: 10 }}
          />
          <Image
            src='https://react.semantic-ui.com/images/wireframe/paragraph.png'
            style={{ marginBottom: 10 }}
          />
          <Image
            src='https://react.semantic-ui.com/images/wireframe/paragraph.png'
            style={{ marginBottom: 10 }}
          />
          <Image
            src='https://react.semantic-ui.com/images/wireframe/paragraph.png'
            style={{ marginBottom: 10 }}
          />
          <Image
            src='https://react.semantic-ui.com/images/wireframe/paragraph.png'
            style={{ marginBottom: 10 }}
          />
          <Image
            src='https://react.semantic-ui.com/images/wireframe/paragraph.png'
            style={{ marginBottom: 10 }}
          />
          <Image src='https://react.semantic-ui.com/images/wireframe/paragraph.png' />
        </Modal.Description>
        </Modal.Content>
        <Modal.Actions>
          <Button onClick={() => setSecondOpen(true)} primary>
            Proceed <Icon name='right chevron' />
          </Button>
        </Modal.Actions>

        <Modal
          onClose={() => setSecondOpen(false)}
          open={secondOpen}
          size='small'
          closeIcon
        >
          <Modal.Header>Modal #2</Modal.Header>
          <Modal.Content>
            <p>That's everything!</p>
          </Modal.Content>
          <Modal.Actions>
            <Button
              icon='check'
              content='All Done'
              onClick={() => setSecondOpen(false)}
            />
          </Modal.Actions>
        </Modal>
      </Modal>
    </>
  )
}

export default ModalExampleMultiple

Expected Result

When opening the nested modal, it should be centered

Actual Result

The nested modal appears at the top of the page and the close icon is cut off

Version

Last version

Testcase

https://codesandbox.io/s/semantic-ui-react-forked-k9cx5?file=/index.js

welcome[bot] commented 3 years ago

👋 Thanks for opening your first issue here! If you're reporting a 🐞 bug, please make sure you've completed all the fields in the issue template so we can best help.

We get a lot of issues on this repo, so please be patient and we will get back to you as soon as we can.

geo-vi commented 3 years ago

Make sure your semantic-ui-css is up to date (latest ver) and that you dont double import at public index.html if you are using CRA

mariajosealvarez commented 2 years ago

Yes, I'm using the last version of semantic-ui-css and semantic-ui-react

brianespinosa commented 2 years ago

This is due to the size of the first modal being so large. There would actually be a significant amount of engineering needed I think to determine how to make a second modal stay centered once a larger modal was on screen that requires controlling. I do not think this time will be invested because the nested modal edge case is a bad UX pattern. There are lots of articles talking about how to remove these patterns from products by doing a quick "nested modals ux" search which will yield a lot of articles with strategies to avoid this pattern.

michaelmarziani commented 2 years ago

@brianespinosa I appreciate the reminder that modals shouldn't be overused, but their merits aside, this framework supports nested modals and the documentation provides examples for them, thus I think they should work.

I can help characterize this problem a bit more as I just ran into it.

At first I had a nested modal where everything was centered, horizontally and vertically as I intended. Then, I noticed that after the content in my first modal got long enough that it would scroll, now the 2nd modal was no longer vertically centered but it was now near the top of the screen.

As a workaround, I set <Modal.Content scrolling> on the 1st modal and now the 2nd modal is again centered, both vertically and horizontally.