Heatmap Reduced List #269
Heatmap Reduced List #269
Conversation
|
@shami-sneha the TS-compile error in |
There was a problem hiding this comment.
Seems to be working fine when I apply the suggestion mentioned up top to wrap the presets file.
Suggestions to address:
- When selecting a heat legend preset in the expanded list (show more = true) that is not in the reduced list,
the selection shows an empty value when going back to the reduced list (show more = false).
Maybe we can add the currently selected heat legend also to the the reduced list (if not present) to avoid showing an empty value in the drop down menu.
| 'simulation-start-day': 'Simulationsstart', | ||
| add_button: 'Fächer', | ||
| }, | ||
| 'group-filters': { | ||
| title: 'Gruppen Verwalten', | ||
| 'title-manage-compartment': 'Fächer verwalten', | ||
| 'nothing-selected': 'Wählen Sie eine Gruppe aus um diese zu bearbeiten oder erstellen Sie eine neue Gruppe.', | ||
| 'add-group': 'Neue Gruppe Erstellen', | ||
| 'add-group-compartment': 'Fächer', | ||
| name: 'Name', |
There was a problem hiding this comment.
These seem to belong to a different PR (#58)
| 'simulation-start-day': 'Simulationsstart', | |
| add_button: 'Fächer', | |
| }, | |
| 'group-filters': { | |
| title: 'Gruppen Verwalten', | |
| 'title-manage-compartment': 'Fächer verwalten', | |
| 'nothing-selected': 'Wählen Sie eine Gruppe aus um diese zu bearbeiten oder erstellen Sie eine neue Gruppe.', | |
| 'add-group': 'Neue Gruppe Erstellen', | |
| 'add-group-compartment': 'Fächer', | |
| name: 'Name', | |
| 'simulation-start-day': 'Simulationsstart', | |
| }, | |
| 'group-filters': { | |
| title: 'Gruppen Verwalten', | |
| 'nothing-selected': 'Wählen Sie eine Gruppe aus um diese zu bearbeiten oder erstellen Sie eine neue Gruppe.', | |
| 'add-group': 'Neue Gruppe Erstellen', | |
| name: 'Name', |
|
|
||
| haetmapchart: { | ||
| textshow: 'Zeig mehr', | ||
| }, |
There was a problem hiding this comment.
There's a typo here preventing the german translation to be used
| haetmapchart: { | |
| textshow: 'Zeig mehr', | |
| }, | |
| heatmapchart: { | |
| textshow: 'Mehr anzeigen', | |
| }, |
| heatmapchart: { | ||
| textshow: 'show more', | ||
| }, |
There was a problem hiding this comment.
All texts should start with a capital letter unless there is a good reason for it not to.
| heatmapchart: { | |
| textshow: 'show more', | |
| }, | |
| heatmapchart: { | |
| textshow: 'Show more', | |
| }, |
| @@ -11,6 +11,7 @@ | |||
| accessibility: 'Accessibility', | |||
| attribution: 'Attribution', | |||
| changelog: 'Changelog', | |||
| 'changelog-path': 'locales/en/changelog-en.md', | |||
There was a problem hiding this comment.
This is a leftover from a merge conflict. PR#294 removed the changelog-path property.
| 'changelog-path': 'locales/en/changelog-en.md', |
| @@ -99,6 +99,7 @@ export default function DistrictMap(): JSX.Element { | |||
|
|
|||
| // fetch geojson | |||
| useEffect(() => { | |||
| // eslint-disable-next-line @typescript-eslint/no-unsafe-argument | |||
There was a problem hiding this comment.
Why is this added?
since it's missing in develop it should run fine without?
| if (heatmapLegends.length === 0 && heatLegendEditOpen) { | ||
| fetch(legendPresets, { | ||
| if (legend.name == 'uninitialized') { | ||
| fetch('assets/heatmap_legend_presets.json', { |
There was a problem hiding this comment.
You have to warp assets for the to work with the changes introduced in PR#249.
Refer to the PR description for further info.
| fetch('assets/heatmap_legend_presets.json', { | |
| fetch('legendPresets', { |
| import {HeatmapLegend} from '../../types/heatmapLegend'; | ||
| import {useTranslation} from 'react-i18next'; | ||
| import legendPresets from 'assets/heatmap_legend_presets.json'; |
There was a problem hiding this comment.
See comment on line 50 and refer to PR#249 for further info.
| import legendPresets from 'assets/heatmap_legend_presets.json'; | |
| import legendPresets from 'assets/heatmap_legend_presets.json'; |
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| }, []); |
There was a problem hiding this comment.
PLease add comments to the dependency list of effects to show when & why the effect should run:
| // eslint-disable-next-line react-hooks/exhaustive-deps | |
| }, []); | |
| // This effect should only run once on first use since the presets don't change. | |
| // eslint-disable-next-line react-hooks/exhaustive-deps | |
| }, []); |
| useEffect(() => { | ||
| if (!activeScenario) { | ||
| return; | ||
| if (activeScenario) { | ||
| dispatch(selectDefaultLegend({selectedScenario: activeScenario - 1})); | ||
| } | ||
|
|
||
| if (legend.name !== 'Default' && legend.name !== 'uninitialized') { | ||
| return; | ||
| } | ||
|
|
||
| selectLegendByName('Default'); | ||
| }, [activeScenario, legend, selectLegendByName]); | ||
| }, [activeScenario, dispatch]); |
There was a problem hiding this comment.
Please add comments to this effect:
| useEffect(() => { | |
| if (!activeScenario) { | |
| return; | |
| if (activeScenario) { | |
| dispatch(selectDefaultLegend({selectedScenario: activeScenario - 1})); | |
| } | |
| if (legend.name !== 'Default' && legend.name !== 'uninitialized') { | |
| return; | |
| } | |
| selectLegendByName('Default'); | |
| }, [activeScenario, legend, selectLegendByName]); | |
| }, [activeScenario, dispatch]); | |
| // Effect to update the default legend when the active scenario changes | |
| useEffect(() => { | |
| if (activeScenario) { | |
| // Select default legend (legends are in a 0-based array while the scenario Ids are 1-based) | |
| dispatch(selectDefaultLegend({selectedScenario: activeScenario - 1})); | |
| } | |
| // This effect should run whenever the active scenrio changes (dispatch should not change during runtime). | |
| }, [activeScenario, dispatch]); |
There was a problem hiding this comment.
Please add comments to your code to make it easier to grasp at a glancec what is happening.
This helps other contributors when they need to handle parts of your code.
Description
language switch
Related Issues
#192
Design Decisions
To reduce heatmap list.
Performance & Quality
Checklist
I, the author of this PR checked the following requirements for good software quality:
I, the reviewer checked the following things: