-
Notifications
You must be signed in to change notification settings - Fork 24
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
psp-9436, require the user to change the advanced filter for it to be… #4434
Conversation
@@ -372,7 +384,12 @@ export const MapStateMachineProvider: React.FC<React.PropsWithChildren<unknown>> | |||
<MapStateMachineContext.Provider | |||
value={{ | |||
mapSideBarViewState: state.context.mapSideBarState, | |||
isShowingSearchBar: !state.context.mapSideBarState.isOpen && !state.context.isFiltering, | |||
isShowingSearchBar: | |||
!state.context.mapSideBarState.isOpen && |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We want to hide the search bar if either the side bar or advanced filter is displayed, or if there is a non-default advanced filter.
The intent behind the original story, was that there is a default filter in effect on the PIMS map at all times. If that default filter is changed, the filter icon should update to display that a non-default filter is in effect. This required that we track the state of the advanced filter globally, with the global having a default value. This actually simplifies the point cluster logic as we are always filtering and not conditionally filtering using the pims visible properties list. The other thing I added in, was to filter any properties that have no visible marker to display on the map, I did this to stop inaccurately cluster counts, which is more noticeable on your local, if you zoom in on clusters with no markers, the cluster count goes down but nothing gets displayed. |
mapFeatureData: state.context.mapFeatureData, | ||
filePropertyLocations: state.context.filePropertyLocations, | ||
pendingFitBounds: state.matches({ mapVisible: { mapRequest: 'pendingFitBounds' } }), | ||
requestedFitBounds: state.context.requestedFitBounds, | ||
isSelecting: state.matches({ mapVisible: { featureView: 'selecting' } }), | ||
isRepositioning: isRepositioning, | ||
selectingComponentId: state.context.selectingComponentId, | ||
isFiltering: state.context.isFiltering, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it is slightly harder to determine if we are filtering now, as in some ways we are "always" filtering. isFiltering is now true if we are using a non-default filter. Perhaps isFiltering could use a rename?
isShowingMapFilter: isShowingMapFilter, | ||
isShowingMapLayers: isShowingMapLayers, | ||
activeLayers: state.context.activeLayers, | ||
activePimsPropertyIds: state.context.activePimsPropertyIds, | ||
showDisposed: state.context.showDisposed, | ||
showRetired: state.context.showRetired, | ||
isMapVisible: state.matches({ mapVisible: {} }), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
required so that the MapContainer doesn't sent the list of visible map properties before the map is loaded, meaning the state machine ignores the event.
const { isFiltering, setVisiblePimsProperties, setShowDisposed, setShowRetired, resetMapFilter } = | ||
useMapStateMachine(); | ||
|
||
const { getMatchingProperties } = usePimsPropertyRepository(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this request logic needed to be moved out of here as this is a conditionally rendered component, and therefore will not run during map load. It needs to run during map load since the default filter takes effect immediately now.
isLoading={getMatchingProperties.loading} | ||
/> | ||
); | ||
if (isShowingMapFilter) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It makes more sense to show this view based on this boolean, not based on whether an advanced filter is active.
@@ -100,38 +101,23 @@ export const PointClusterer: React.FC<React.PropsWithChildren<PointClustererProp | |||
|
|||
const pimsLocationFeatures: FeatureCollection<Geometry, PIMS_Property_Location_View> = | |||
useMemo(() => { | |||
if (mapMachine.isFiltering && mapMachine.mapFeatureData.pimsLocationFeatures !== null) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
don't need this if/else anymore because the list of activePimsPropertyIds is always active now.
}; | ||
} | ||
// Do not cluster any points that do not have markers on the map. | ||
const displayableFeatures = filteredFeatures.filter(f => exists(getMarkerIcon(f, false))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as discussed in my summary, this prevents "inaccurate" cluster counts.
4088f86
to
1f6565d
Compare
this PR is showing a lot of conflicts - might be worth to rebase the branch |
1f6565d
to
47730fd
Compare
See CodeCov Report Here: https://app.codecov.io/github/bcgov/psp/pull/4434 |
… applied. requires tests.
… on". - Update point clusterer to not cluster properties that are unable to display a marker.
47730fd
to
b395b9d
Compare
See CodeCov Report Here: https://app.codecov.io/github/bcgov/psp/pull/4434 |
* psp-9235 | Contact Navigation check (#4416) * improved logic to contacts navigation checking --------- Co-authored-by: Manuel Rodriguez <[email protected]> * CI: Bump version to v5.6.0-91.24 * Version bump IS92 (#4418) * Bump DEV version * Lint fixes * CI: Bump version to v5.6.0-92.1 * PSP-9268 Add sub-interest files to a main file (#4413) * Inject parentId into create-acq-file route * Update container and form to copy information from parent file to sub-files on creation * Improve JS typing * Add sub-interest files * Fix bug in model mappings * Changes to navigation from sub-files * Fix bug when copying acquisition team from parent file into sub-file * Backend changes to support adding sub-files * Hide button for sub-files * Fix bug creating sub-files * Ask user for confirmation when changing main file project or product * Test updates * Fix merge conflicts * Test updates * PR feedback * Fix typing error after merging another PR * Update snapshots * CI: Bump version to v5.6.0-92.2 * PSP-9351 UI/UX: The close button at the top right side of the sub-menu should be integrated within the header (#4414) * PSP-9351 UI changes to side tray close button for consistency * Update snapshots * Test updates * Update snapshots * CI: Bump version to v5.6.0-92.3 * PSP-9411 : FT-REG: Lease & Licences: Header still displays "Tenant" when the lease is payable. (#4419) Co-authored-by: Herrera <[email protected]> Co-authored-by: Alejandro Sanchez <[email protected]> * CI: Bump version to v5.6.0-92.4 * Fixed repository not updating doc purpose (#4420) * CI: Bump version to v5.6.0-92.5 * IS-92.00 Database Schema (#4408) * IS-92.00 Database Schema PSP_PIMS | Development Sprint: S92.00 | Design Sprint: 91 | Date: 2024-Oct-21 - Added tables: - PIMS_DOCUMENT_QUEUE - PIMS_DOCUMENT_QUEUE_HIST - PIMS_DOCUMENT_QUEUE_STATUS_TYPE - PIMS_SUBFILE_INTEREST_TYPE - Altered tables: - PIMS_ACQUISITION_FILE - PIMS_ACQUISITION_FILE_HIST - PIMS_DOCUMENT - PIMS_DOCUMENT_HIST - Added seed script: - 049_DML_PIMS_DATA_SOURCE_TYPE.sql - 157_DML_PIMS_SUBFILE_INTEREST_TYPE.sql - 158_DML_PIMS_DOCUMENT_QUEUE_STATUS_TYPE.sql - Requires additional metadata to meet standards * Added Lease/License Alters - Regenerated master.sql * Changed comments to work with current settings --------- Co-authored-by: Manuel Rodriguez <[email protected]> * CI: Bump version to v5.6.0-92.6 * Generated scaffold (#4421) * CI: Bump version to v5.6.0-92.7 * psp-9186 prevent merging of pims and parcel map data - use pims if exists, otherwise use parcel map. (#4412) Co-authored-by: Alejandro Sanchez <[email protected]> * CI: Bump version to v5.6.0-92.8 * Changes on automation branch IS92 (#4425) * CI: Bump version to v5.6.0-92.9 * Fix faulty logic when closing property info panel (#4424) * CI: Bump version to v5.6.0-92.10 * PSP-9406 fix * PSP-9252 Add date fields to acquisition file (major projects) (#4428) * Model updates * Include new fields in Create Acquisition form * Show new fields on acquisition detail view * Capture new fields in Update acquisition form * Test updates * Update snapshots * CI: Bump version to v5.6.0-92.11 * PSP-9089 Warn user if they are about to lose data when cancelling "Add Document" (#4422) * Display confirmation message before closing document upload modal * Inject view into document upload container * Code cleanup * Update snapshots * Test updates * PR feedback * CI: Bump version to v5.6.0-92.12 * PSP-9341 : Sub-file interest type (#4423) * PSP-9341 : Sub-file interest type * - test updates * - updates * - updates * - test updates --------- Co-authored-by: Herrera <[email protected]> Co-authored-by: Alejandro Sanchez <[email protected]> * CI: Bump version to v5.6.0-92.13 * PSP-9331: UI UX Clean Up - Replace notes field (#4429) * Changes made * Update snapshots and unit tests * Disposition notes and Property Details Yup updated --------- Co-authored-by: devinleighsmith <[email protected]> Co-authored-by: Alejandro Sanchez <[email protected]> * CI: Bump version to v5.6.0-92.14 * PSP-8596 : UI UX Clean Up - Menu order (#4430) * PSP-8596 : UI UX Clean Up - Menu order * PSP-8596 : UI UX Clean Up - Menu order --------- Co-authored-by: Herrera <[email protected]> Co-authored-by: Alejandro Sanchez <[email protected]> * CI: Bump version to v5.6.0-92.15 * PSP-9205 : UI UX Clean Up - tab menu - color replacement (#4426) Co-authored-by: Herrera <[email protected]> * CI: Bump version to v5.6.0-92.16 * PSP-9412 fix * CI: Bump version to v5.6.0-92.17 * CI: Bump version to v5.6.0-92.18 * PSP-9465 Sub interest section is missing on the view screen of Sub-interest file (#4438) * Add missing sub-interest section to acquisition sub-files * Test updates * CI: Bump version to v5.6.0-92.19 * PSP-9435 : FT-REG: Lease & Licences: Payee Header is not refreshed as soon as the user deletes the payee (#4440) Co-authored-by: Herrera <[email protected]> * CI: Bump version to v5.6.0-92.20 * PSP-9205 : UI UX Clean Up - tab menu - color replacement (#4441) Co-authored-by: Herrera <[email protected]> * CI: Bump version to v5.6.0-92.21 * psp-9466 ensure that the full pims location data is included in the map marker click event - since the minimal layer does not have all necessary properties. (#4445) Signed-off-by: devinleighsmith <[email protected]> * CI: Bump version to v5.6.0-92.22 * PSP-9474 : FT: Acquisition File: Sub-interest File: For Other file interest type Placeholder text is missing on the free text box (#4442) Co-authored-by: Herrera <[email protected]> * CI: Bump version to v5.6.0-92.23 * Adding tooltip of Acquisition File Create form (#4447) * CI: Bump version to v5.6.0-92.24 * Updated sync helper to work with new format of files. Sorting before creating json to help in comparing. Other fixes (#4444) Co-authored-by: Alejandro Sanchez <[email protected]> * CI: Bump version to v5.6.0-92.25 * PSP-9302 Leases Compensation: 500 error should throw a friendlier message when a user attempts to delete a stakeholder that is associated to a compensation (#4449) * WIP * Fix typo * PSP-9302 Provide user-friendly message * Reset lease stakeholder state when API responds with error 409 * Test updates * Fix merge conflicts * CI: Bump version to v5.6.0-92.26 * PSP-8720 UI-UX cleanup - Disabled Buttons (#4436) * UI UX clean up for disabled buttons * Update snapshots * CI: Bump version to v5.6.0-92.27 * Psp 9318 - document queue base etl (#4443) * psp-9318 pat_pims_document_import etl wip. * update to new PIMS table. * psp-9317 create base ETL to import documents from TAP. * revert password. --------- Co-authored-by: Alejandro Sanchez <[email protected]> * CI: Bump version to v5.6.0-92.28 * PSP-8759 : Creating and Editing Property Records – Property Address F… (#4433) * PSP-8759 : Creating and Editing Property Records – Property Address Fields * - test updates * - test updates --------- Co-authored-by: Herrera <[email protected]> Co-authored-by: Alejandro Sanchez <[email protected]> * CI: Bump version to v5.6.0-92.29 * PSP-8569 : UI UX Clean Up - Properties Menu (Acquisition, Disposition… (#4437) * PSP-8569 : UI UX Clean Up - Properties Menu (Acquisition, Disposition & Research File) * - updates * Update snapshots --------- Co-authored-by: Herrera <[email protected]> Co-authored-by: Alejandro Sanchez <[email protected]> * CI: Bump version to v5.6.0-92.30 * Psp 9405 allow layers to be refreshed programatically. (#4439) * psp-9405 allow updates that cause changes to map layers to be refreshed * formatting corrections. * mock correction. * code review corrections. * CI: Bump version to v5.6.0-92.31 * psp-9436, require the user to change the advanced filter for it to be… (#4434) * psp-9436, require the user to change the advanced filter for it to be applied. requires tests. * psp-9436 change advanced filter so that the default filter is "always on". - Update point clusterer to not cluster properties that are unable to display a marker. * re-sort dependencies --------- Co-authored-by: Smith <[email protected]> * CI: Bump version to v5.6.0-92.32 * PSP-9488 Titles are missing on Properties links and File Summary (#4456) * Titles are missing on Properties links and File Summary * Update snapshots * CI: Bump version to v5.6.0-92.33 * PSP-9489 : FT: Property Details: The default option for the Province should be "British Columbia" (#4454) Co-authored-by: Herrera <[email protected]> Co-authored-by: Alejandro Sanchez <[email protected]> * CI: Bump version to v5.6.0-92.34 * correct inconsistent reset of advanced filter. (#4455) * correct inconsistent reset of advanced filter. * test correction. --------- Co-authored-by: Alejandro Sanchez <[email protected]> * CI: Bump version to v5.6.0-92.35 * Updated repository to only retrieve the necessary information for a report (#4457) * CI: Bump version to v5.6.0-92.36 --------- Signed-off-by: devinleighsmith <[email protected]> Co-authored-by: devinleighsmith <[email protected]> Co-authored-by: Sue Tairaku <[email protected]> Co-authored-by: Manuel Rodriguez <[email protected]> Co-authored-by: Manuel Rodriguez <[email protected]> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Eduardo <[email protected]> Co-authored-by: Herrera <[email protected]> Co-authored-by: Doug Filteau <[email protected]> Co-authored-by: Sue Tairaku <[email protected]> Co-authored-by: Smith <[email protected]>
… applied.
requires tests.