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.

Sign up for GitHub

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

Jump to bottom

EES-5541 - Boundary level per dataset form changes #5308

Merged
merged 18 commits into from
Nov 26, 2024
Merged

EES-5541 - Boundary level per dataset form changes #5308

merged 18 commits into from
Nov 26, 2024

Conversation

bennettstuart
Copy link
Collaborator

@bennettstuart bennettstuart commented Oct 4, 2024 •
edited
Loading

Overview

Most changes are hidden behind a feature flag.

This work introduces the concept of boundaryLevels on dataSetConfigs on maps, this is to allow maps to show different geoJson per dataset as to produce a more accurate representation of data alongside their boundaries on the map.

  • Reducer actions altered to cater for dataset boundary levels
  • Separated Chart boundary levels form data setup and boundary levels form into their own components for simplicity.
  • (hidden for now) data set table with boundary level select, below existing boundary level select, to allow users to select boundary levels per data set
  • Refactor to chartBuilderReducer:
    • Hoists initialization of map dataSetConfigs into chartBuilderReducer
    • Refactors form types for boundary level and data grouping tabs
    • Fixes boundary level tab not showing invalid form errors for other tabs
    • Refactors out new getMapDataSetCategoryConfigs function from getDataSetCategoryConfigs

Additional

  • Added onChange prop to our Form component
    • <Effect value={formValues} onChange={props.onChange} /> no longer the only option to onChange form updates (and it doesn't work with form array field)
  • Fixed a bug on Chart data groupings tab not allowing modal to close

image

@bennettstuart bennettstuart force-pushed the EES-5402--EES-5541-form-amends branch 2 times, most recently from a6803f3 to 39ac02c Compare October 15, 2024 08:42
@bennettstuart bennettstuart force-pushed the EES-5402--EES-5541-form-amends branch 2 times, most recently from ef99d0f to 1b744fc Compare October 15, 2024 09:10
@bennettstuart bennettstuart marked this pull request as ready for review October 16, 2024 12:10
@bennettstuart bennettstuart force-pushed the EES-5402--EES-5541-form-amends branch from 6fead8c to f1fb1ba Compare October 16, 2024 12:45
@bennettstuart bennettstuart changed the base branch from EES-5402 to dev October 16, 2024 12:45
@bennettstuart bennettstuart changed the title EES-5402⬅️EES-5541 - Boundary level per dataset form changes EES-5541 - Boundary level per dataset form changes Oct 16, 2024
@bennettstuart bennettstuart requested a review from ntsim October 16, 2024 13:19
@bennettstuart bennettstuart force-pushed the EES-5402--EES-5541-form-amends branch from c17d345 to 13bcc41 Compare October 16, 2024 13:36
@bennettstuart bennettstuart marked this pull request as draft October 30, 2024 16:06
@bennettstuart bennettstuart marked this pull request as ready for review November 5, 2024 14:24
@bennettstuart bennettstuart force-pushed the EES-5402--EES-5541-form-amends branch from b6c2967 to ce726ef Compare November 5, 2024 14:28
ntsim
ntsim requested changes Nov 6, 2024
View reviewed changes
...ics-admin/src/pages/release/datablocks/components/chart/ChartBoundaryLevelsConfiguration.tsx Outdated Show resolved Hide resolved
...n-statistics-admin/src/pages/release/datablocks/components/chart/ChartBoundaryLevelsForm.tsx Outdated Show resolved Hide resolved
...ages/release/datablocks/components/chart/__tests__/ChartBoundaryLevelsConfiguration.test.tsx Outdated Show resolved Hide resolved
...ages/release/datablocks/components/chart/__tests__/ChartBoundaryLevelsConfiguration.test.tsx Outdated Show resolved Hide resolved
...ages/release/datablocks/components/chart/__tests__/ChartBoundaryLevelsConfiguration.test.tsx Outdated Show resolved Hide resolved
...ion-statistics-common/src/modules/charts/util/__tests__/getMapDataSetCategoryConfigs.test.ts Outdated Show resolved Hide resolved
...ion-statistics-common/src/modules/charts/util/__tests__/getMapDataSetCategoryConfigs.test.ts Outdated Show resolved Hide resolved
...pages/release/datablocks/components/chart/__tests__/ChartDataGroupingsConfiguration.test.tsx Show resolved Hide resolved
...n-statistics-admin/src/pages/release/datablocks/components/chart/ChartBoundaryLevelsForm.tsx Show resolved Hide resolved
src/explore-education-statistics-common/src/components/form/Form.tsx Show resolved Hide resolved
@bennettstuart bennettstuart force-pushed the EES-5402--EES-5541-form-amends branch from 7e7338c to 05f7c73 Compare November 11, 2024 16:13
@bennettstuart bennettstuart requested a review from ntsim November 11, 2024 16:18
@bennettstuart bennettstuart force-pushed the EES-5402--EES-5541-form-amends branch from 05f7c73 to 9b14c0d Compare November 11, 2024 16:19
ntsim
ntsim requested changes Nov 13, 2024
View reviewed changes
...ages/release/datablocks/components/chart/__tests__/ChartBoundaryLevelsConfiguration.test.tsx Outdated Show resolved Hide resolved
...ages/release/datablocks/components/chart/__tests__/ChartBoundaryLevelsConfiguration.test.tsx Outdated Show resolved Hide resolved
...ages/release/datablocks/components/chart/__tests__/ChartBoundaryLevelsConfiguration.test.tsx Outdated Show resolved Hide resolved
...ages/release/datablocks/components/chart/__tests__/ChartBoundaryLevelsConfiguration.test.tsx Show resolved Hide resolved
...pages/release/datablocks/components/chart/__tests__/ChartDataGroupingsConfiguration.test.tsx Show resolved Hide resolved
...src/pages/release/datablocks/components/chart/reducers/__tests__/chartBuilderReducer.test.ts Show resolved Hide resolved
...src/pages/release/datablocks/components/chart/reducers/__tests__/chartBuilderReducer.test.ts Outdated Show resolved Hide resolved
src/explore-education-statistics-common/src/components/form/__tests__/Form.test.tsx Outdated Show resolved Hide resolved
src/explore-education-statistics-common/src/components/form/__tests__/Form.test.tsx Outdated Show resolved Hide resolved
src/explore-education-statistics-common/src/components/form/__tests__/Form.test.tsx Show resolved Hide resolved
@bennettstuart bennettstuart force-pushed the EES-5402--EES-5541-form-amends branch 2 times, most recently from e08b848 to 10e8149 Compare November 21, 2024 11:08
@bennettstuart bennettstuart requested a review from ntsim November 21, 2024 11:15
ntsim
ntsim reviewed Nov 22, 2024
View reviewed changes
...atistics-admin/src/pages/release/datablocks/components/chart/__tests__/ChartBuilder.test.tsx Outdated Show resolved Hide resolved
...atistics-admin/src/pages/release/datablocks/components/chart/__tests__/ChartBuilder.test.tsx Outdated Show resolved Hide resolved
...atistics-admin/src/pages/release/datablocks/components/chart/__tests__/ChartBuilder.test.tsx Show resolved Hide resolved
...ages/release/datablocks/components/chart/__tests__/ChartBoundaryLevelsConfiguration.test.tsx Outdated Show resolved Hide resolved
...ages/release/datablocks/components/chart/__tests__/ChartBoundaryLevelsConfiguration.test.tsx Outdated Show resolved Hide resolved
...src/pages/release/datablocks/components/chart/reducers/__tests__/chartBuilderReducer.test.ts Outdated Show resolved Hide resolved
...src/pages/release/datablocks/components/chart/reducers/__tests__/chartBuilderReducer.test.ts Outdated Show resolved Hide resolved
...src/pages/release/datablocks/components/chart/reducers/__tests__/chartBuilderReducer.test.ts Outdated Show resolved Hide resolved
...src/pages/release/datablocks/components/chart/reducers/__tests__/chartBuilderReducer.test.ts Show resolved Hide resolved
...src/pages/release/datablocks/components/chart/reducers/__tests__/chartBuilderReducer.test.ts Outdated Show resolved Hide resolved
@bennettstuart bennettstuart force-pushed the EES-5402--EES-5541-form-amends branch 2 times, most recently from f824433 to cd5c749 Compare November 22, 2024 15:17
@bennettstuart bennettstuart requested a review from ntsim November 22, 2024 15:19
@bennettstuart bennettstuart force-pushed the EES-5402--EES-5541-form-amends branch from cd5c749 to 6bb4792 Compare November 22, 2024 15:20
ntsim
ntsim approved these changes Nov 22, 2024
View reviewed changes
Copy link
Collaborator

@ntsim ntsim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

bennettstuart and others added 17 commits November 22, 2024 16:55
@bennettstuart @ntsim
EES-5541 add dataset boundary level configuration to form
61ad014
@bennettstuart @ntsim
EES-5541 data groupings tab form init massively simplified, ammended …
9ea3a48 
…data set boundary form interaction and select
@bennettstuart @ntsim
EES-5541 - wired up forms to reducer state
6013ad8
@bennettstuart @ntsim
EES-5541 simplified form layout and updated reducer
c308f57
@bennettstuart @ntsim
EES-5541 added onChange to Form and simplified boundary form implemen…
2c5032d 
…tation
@bennettstuart @ntsim
EES-5541 added feature flag, fixed build errors
e949e49
@ntsim
EES-5541 Various fixes across boundary level and data groupings
6c622d8 
- Hoists initialization of map `dataSetConfigs` into `chartBuilderReducer`
- Refactors form types for boundary level and data grouping tabs
- Fixes boundary level tab not showing invalid form errors for other tabs
- Refactors out new `getMapDataSetCategoryConfigs` function from
  `getDataSetCategoryConfigs`.
@bennettstuart @ntsim
EES-5541 fixes stale boundaryLevel form state bug when removing datasets
2bd5569
@bennettstuart @ntsim
EES-5541 removed defaultBoundaryLevel, which lead to unset configs be…
cd1d9a8 
…coming set
@bennettstuart @ntsim
EES-5541 updated getDataSetCategoryConfigs tests, no longer used prop…
79adfca 
…s removed etc.
@bennettstuart @ntsim
EES-5541 added 2 simple tests for getMapDataSetCategoryConfigs
91923d6
@bennettstuart @ntsim
EES-5541 generateFeaturesAndDataGroups: fixed types
1de6686
@bennettstuart @ntsim
EES-5541 fix up test:ChartBoundaryLevelsConfiguration
1c65300
@bennettstuart @ntsim
EES-5541 mostly fixed tests: ChartDataGroupingsConfiguration
90f63b5
@bennettstuart @ntsim
EES-5541 chartBuilderReducer tests fix
e31528b
@bennettstuart @ntsim
EES-5541 ChartDataGroupingsConfiguration tests fix
7a01fdb
@bennettstuart @ntsim
EES-5541 various typo fixes and test improvements
bb6998e
@ntsim ntsim force-pushed the EES-5402--EES-5541-form-amends branch from 6bb4792 to bb6998e Compare November 22, 2024 16:55
@ntsim ntsim merged commit d538755 into dev Nov 26, 2024
8 checks passed
@ntsim ntsim deleted the EES-5402--EES-5541-form-amends branch November 26, 2024 02:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ntsim ntsim ntsim approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@bennettstuart @ntsim