Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FLAG-343] fire alert widget crashes page when clicking settings button #4703

Conversation

SARodrigues
Copy link
Collaborator

@SARodrigues SARodrigues commented Oct 11, 2023

Overview

Unlike most other widgets where the widget settings are predefined, in this widget (firesAlertsSimple) the min/max/start dates for the datepickers in the settings are determined based on the dates fetched from an endpoint.

The integratedDeforestationAlerts widget also works in a similar fashion and suffers from the same bug; although it fetches data from a glad endpoint instead.

From what I learned during the debugging session, when the user opens the settings dropdown before the dates are available to populate the datepickers, the datepickers crash (undefined dates).


I've considered a few options:

  1. Implement a mechanism to prevent the settings dropdown from opening until all data is available
    though this would mean re-engineering part of the core of the components that render the widgets in order to allow settings to be set asynchronously, which seems a bit overkill for just a couple outlier widgets
  2. Hiding the settings button/dropdown while the widget is in a loading state
    however, this would cause the button to flicker when the widget is loading data to display in its body, such as when the user changes its settings
  3. Hardcode dates in the widget's settings that the datepickers can use while the necessary data isn't returned by the endpoints
    This seemed like a flakey solution and more of a patch; which I'm not sure how it'd influence the widget's data calculations
  4. Make the crashing datepickers themselves "fail gracefully".
    in this case, if there aren't valid dates to be displayed yet, then the datepicker buttons show a loading spinner.

This PR implements option 4, as it seems like the easier and safer approach; any other widgets that happen to have the same bug and have been undetected should now not trigger the bug either.


Notes:

  • I've found a prop being passed that was unused here, which I took the chance to remove in order to prevent confusion in the future
  • I've also noticed that the WidgetHeader component did expect a loading prop, which I've passed here. It is used mainly to disable the dropdowns in the widget's settings while the widget is loading.
  • Other changes were just auto linting/prettier

Testing

  1. Visit this link
  2. Before the "FIRE ALERTS IN AREA IN BAHIA, BRAZIL" widget finishes loading, click the "Settings" button
  3. Verify that the Dashboard no longer crashes

Tracking

FLAG-343

@SARodrigues SARodrigues force-pushed the FLAG-343-fire-alert-widget-crashes-page-when-clicking-settings-button branch from a19a4df to 1a4e29e Compare November 9, 2023 15:18
@SARodrigues SARodrigues temporarily deployed to gfw-staging-pr-4703 November 9, 2023 15:18 Inactive
@SARodrigues SARodrigues marked this pull request as ready for review November 9, 2023 15:37
@SARodrigues SARodrigues merged commit 2daa452 into develop Nov 9, 2023
4 checks passed
@SARodrigues SARodrigues deleted the FLAG-343-fire-alert-widget-crashes-page-when-clicking-settings-button branch November 9, 2023 15:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants