yalla-coop / curenetics

A platform enabling medical professionals to more quickly find relevant clinical trials for their patients
2 stars 0 forks source link

Small modal bug #159

Closed wright1 closed 4 years ago

wright1 commented 4 years ago

This branch relates to issue #131.

thejoefriel commented 4 years ago

The modal still has some issues:

Here's a video showing how it looks on my screen: https://www.loom.com/share/574d4564d5fa437c9720d154ff06e9ab

wright1 commented 4 years ago

I didn't want to play with those margins because they are created by globalStyles. But I will adjust if need be. Also when you say margin on top, do you mean the header/nav bar? Do you want that grayed out too? Making the header fixed could solve the header issue but that may have knock on effects so I'm not keen to do that.

thejoefriel commented 4 years ago

I didn't want to play with those margins because they are created by globalStyles. But I will adjust if need be. Also when you say margin on top, do you mean the header/nav bar? Do you want that grayed out too? Making the header fixed could solve the header issue but that may have knock on effects so I'm not keen to do that.

Hi @wright1 - unfortunately we can't have a margin down the left side of the screen as it looks like a bug. Typically with any modal it grays out the entire screen behind it. For an example, here's the antd modal that we first recommended using (select 'open modal' on this page and you can see): https://ant.design/components/modal/

Regarding the navbar, typically with a modal the whole screen is grayed out, but if you at least moved the modal down so it wasn't flush with the navbar then it would look better, and you don't need to gray out the navbar if it's causing a blocker.

In terms of overriding styles, I had a quick look myself and you can override the globalStyles on this by using position: absolute on the section and then position: fixed on the modal . So for example something like this:

const Bkground = styled.section`
  /* width: 100vw;
  height: 100vh;*/
  position: absolute;
  z-index: 1;
  left: 0;
  top: 0;
  /* background-color: rgba(255, 255, 255, 0.4); */
  width: 100%;
  position: absolute;
  height: 100vh;
  background-color: rgba(255, 255, 255, 0.4);
  display: flex;
  align-items: center;
  justify-content: center;

  div {
    width: 70vw;
    position: fixed;
    height: auto;
    padding: 2vw;
    background-color: ${colors.white};
    border: 3px solid ${colors.accent};
    border-radius: 5px;
    z-index: 2;
    @media (max-width: 450px) {
      height: 90vh;
      width: 90vw;
      padding-left: 3vw;
      position: absolute;
      overflow: scroll;
    }
  }
`;

This is the result: https://www.loom.com/share/e4efad1fdabb4d3b90831cbc134eb861

As you can see it grays out the whole screen and works across desktop and mobile. As I say though this is something I did very quickly so there may be ways to improve this further, but as you can see there's no longer any margin on the left, the screen is greyed out and the modal is fixed in the center of the screen

I also noticed that there's a common modal but then this upload page uses it's own individual modal. Probably something ti refactor in the future - no need right now - but considering there's two, just a heads up to double check that you make the style fixes to all the modals on the app.