Skip to content
This repository has been archived by the owner on Aug 28, 2023. It is now read-only.

Models modal doesn't close with the related indicators modal #660

Closed
alexelash opened this issue Feb 21, 2018 · 2 comments · Fixed by #757
Closed

Models modal doesn't close with the related indicators modal #660

alexelash opened this issue Feb 21, 2018 · 2 comments · Fixed by #757

Comments

@alexelash
Copy link
Contributor

User can end up with just an overlay and will have to refresh to be able to access any part of the app.

Steps to reproduce:

  1. Open "Related indicators" modal
  2. Open an indicator's accordian
  3. Select the "Models" modal
  4. Click the close button of the Related indicators modal.

Expected behavior: hitting the close button of the related indicators modal closes both modals, including their respective overlays.

@flibbertigibbet
Copy link
Contributor

One approach might be for the parent modal to send an event when it closes that the child modal listens for, and also closes itself. This would either require the indicator component be able to find a reference to its child modal, if open, at the time the parent closes, or else modify the component library for the child modal itself to listen for the event.

@caseycesari
Copy link
Contributor

caseycesari commented Feb 26, 2018

I spent a good chunk of time looking into this, and ran into several problems, which are detailed below.

My first approach was to get access to the child ModelModalComponent in IndicatorChartComponent, and use the close method on the modal when the indicator chart parent modal was dismissed. However, I was not able to import the ModelModalComponent from the CCC library. I don’t think it’s exported in a way that allows it to be accessed in component code (as opposed to a component template). I tried importing ChartModule into the component and making references to ModelModalComponent, but the model component could not be found. I also tried various permutations of the following import syntax:

import { ModelModalComponent } from 'climate-change-components/charts.module'

I was unable to figure out an import syntax that worked. @CloudNiner suggested importing it directly like so:

import { ModelModalComponent } from 'climate-change-components/src/lib/modules/charts/components/model-modal/model-modal.component.ts'

This appeared to work, but I than ran into two other problems. First, using @ViewChild, the model modal component came back as undefined. I thought this was because the component isn’t initially displayed when the chart is opened. However, the component consists of a button which is displayed on initialization, so this should work. In any case, when trying to build the project, I got the following error:

ERROR in Error: Cannot determine the module for class ModelModalComponent in /opt/planit/angular/planit/node_modules/climate-change-components/src/lib/modules/charts/components/model-modal/model-modal.component.ts! Add ModelModalComponent to the NgModule to fix it.

I think we likely have to modify the CCC library to export the model modal component in a different way.

I then paired with @flibbertigibbet, who came up with a simpler solution that we investigated. Based on the behavior of the nested modals in the ngx-bootstrap documentation, clicking anywhere outside of the child modal should close it instead of the click propagating to the parent modal (and therefore creating the current bug). However, clicking to close wasn’t working on the related indicators model, even though it wasn’t specifically disabled (like it is for the “Add hazards” modal). It turns out some CSS rules were preventing this from happening (update: @lederer looked into this and provided a fix). After removing all of the modal CSS rules from the project, clicking to close modals worked. However, if the model modal is open, you can only click within the bounds of the parent modal to have model modal close. Clicking outside of the parent modal closes the parent modal, even though it should, according to the demo, close the child modal first. We think this might have something to do with how the modals are structured in the DOM. In the documentation, the modals are siblings. In the app, the model modal (child) is far down in the DOM, while the related indicator modal (parent) is attached to the body. Specifying a target element to attach the modal to is not yet supported.

Next I will try the approach @flibbertigibbet details above.

caseycesari added a commit to azavea/climate-change-components that referenced this issue Mar 2, 2018
Instead of updating the model selection every time the modal is closed,
update the selection when the update button is clicked. This prevents
errors related to trying to update the model selection when the parent
chart has been removed from the DOM.

Also, expose a public method to close the modal. This can be used to
close the modal without triggering any updates to the models.

Refs azavea/temperate#660
caseycesari added a commit that referenced this issue Mar 2, 2018
If the parent modal of the indicator charts is closed, close the model
modal. Because the parent modal isn't a direct parent of the indicator
chart, we check the visibility of the chart each time Angular checks for
changes to components. If the chart isn't visible anymore, close the
model modal. This ensures the model modal isn't left open when the chart
is closed, which 1) left an extra modal backdrop open over the app and
2) caused additional problems when the indicator charts were reopened.

Refs #660
caseycesari added a commit that referenced this issue Mar 5, 2018
If the parent modal of the indicator charts is closed, close the model
modal. Because the parent modal isn't a direct parent of the indicator
chart, we check the visibility of the chart each time Angular checks for
changes to components. If the chart isn't visible anymore, close the
model modal. This ensures the model modal isn't left open when the chart
is closed, which 1) left an extra modal backdrop open over the app and
2) caused additional problems when the indicator charts were reopened.

Refs #660
caseycesari added a commit that referenced this issue Mar 5, 2018
If the parent modal of the indicator charts is closed, close the model
modal. Because the parent modal isn't a direct parent of the indicator
chart, we check the visibility of the chart each time Angular checks for
changes to components. If the chart isn't visible anymore, close the
model modal. This ensures the model modal isn't left open when the chart
is closed, which 1) left an extra modal backdrop open over the app and
2) caused additional problems when the indicator charts were reopened.

Refs #660
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants