Wire up boundary details page #143

mstone121 · 2022-10-19T21:28:56Z

Overview

This PR wires up the boundary details page. Some of the data on this page was not present in the data schema so that has been updated as well.

Closes #89

Notes

I not entirely sure what to do about the activity log. It needs more formatting/syntax to match what is in figma. I think this functionality should all be in one place (and not split between the front end and back end).

Testing Instructions

Login as a contributor
View some boundaries
- Ensure "Review" button does not appear for boundaries in review or approved
Login as validator
View some boundaries
- Ensure "Edit" button does not appear for drafts

Checklist

fixup! commits have been squashed
CHANGELOG.md updated with summary of features or fixes, following Keep a Changelog guidelines
README.md updated if necessary to reflect the changes
CI passes after rebase

rajadain · 2022-10-20T17:54:22Z

Will take a look at this after merge conflicts have been fixed

mstone121 · 2022-10-20T18:07:59Z

@rajadain Conflicts resolved, ready for review

rajadain · 2022-10-20T18:09:10Z

Taking a look

rajadain

This is headed in the right direction. A few more changes needed to match Figma.

I think this functionality should all be in one place (and not split between the front end and back end).

I agree. Does what the back-end send enough to have this formatting code in the front-end? What would you recommend changing about the back-end response?

src/django/api/models/submission.py

src/app/src/components/Submissions/Detail/Detail.js

src/app/src/components/Submissions/Detail/Map.js

src/django/api/models/user.py

src/app/src/components/Submissions/Detail/Map.js

src/app/src/components/Submissions/Badges.js

src/app/src/components/Submissions/ActivityLog.js

mstone121

@rajadain Ready for review again. See comments

src/django/api/models/submission.py

src/django/api/models/user.py

src/app/src/components/Submissions/Detail/Detail.js

src/app/src/components/Submissions/Detail/Map.js

src/app/src/components/Submissions/Badges.js

src/app/src/components/Submissions/ActivityLog.js

rajadain

Great work on this! Just a couple minor adjustments more, otherwise this is good to go.

src/app/src/components/Submissions/Detail/Map.js

rajadain · 2022-10-24T14:11:01Z

Also seeing this behavior where the background is white on scrolling down. Going to take a quick stab at this.

2022-10-24.09.56.21.mp4

rajadain · 2022-10-24T14:20:35Z

the background is white on scrolling down

This fixes it:

diff --git a/src/app/src/components/Submissions/Detail/Detail.js b/src/app/src/components/Submissions/Detail/Detail.js
index 5b044c6..b91af9b 100644
--- a/src/app/src/components/Submissions/Detail/Detail.js
+++ b/src/app/src/components/Submissions/Detail/Detail.js
@@ -18,6 +18,7 @@ import Info from './Info';
 import Map from './Map';
 import { useEndpointToastError } from '../../../hooks';
 import { useStartReviewMutation } from '../../../api/reviews';
+import { NAVBAR_HEIGHT } from '../../../constants';
 
 export default function SubmissionDetail() {
     const navigate = useNavigate();
@@ -56,7 +57,7 @@ export default function SubmissionDetail() {
             bg='gray.50'
             align='stretch'
             divider={<StackDivider />}
-            h='100vh'
+            minH={`calc(100vh - ${NAVBAR_HEIGHT}px)`}
         >
             <Flex mb={7}>
                 <Flex direction='column' w='50%'>

rajadain

Actually there are a couple missing features:

When the Boundary is in Needs Revisions, the Contributor should be able to "Revise boundary". Also the info box should be orange, not green.
Current:

Wireframe:
For Validators, there should no info box. Also, they should have an Approve button in the top right.
Wireframe:

mstone121 · 2022-10-24T16:39:15Z

src/app/src/components/Submissions/Detail/Map.js

+            case BOUNDARY_STATUS.NEEDS_REVISIONS:
+                return getReviewDetailText(boundary.submission.review);
+            case BOUNDARY_STATUS.APPROVED:
+                return 'Your map has been approved.';


Changed from this to your to match Figma.

mstone121 · 2022-10-24T16:42:20Z

src/app/src/components/Submissions/Detail/Map.js

+    const getBarColor = () => {
+        switch (boundary.status) {
            case BOUNDARY_STATUS.SUBMITTED:
+                return 'teal';
            case BOUNDARY_STATUS.IN_REVIEW:
-                return 'This map will be reviewed.';
            case BOUNDARY_STATUS.NEEDS_REVISIONS:
-                return 'This map needs revisions.';
+                return 'orange';
            case BOUNDARY_STATUS.APPROVED:
-                return 'This map has been approved.';
+                return 'green';
            default:
-                throw new Error(`Unexpected status ${status}`);
+                return null;
        }
    };


This diff is confusing. getBarColor looks like this

const getBarColor = () => { switch (boundary.status) { case BOUNDARY_STATUS.SUBMITTED: return 'teal'; case BOUNDARY_STATUS.IN_REVIEW: case BOUNDARY_STATUS.NEEDS_REVISIONS: return 'orange'; case BOUNDARY_STATUS.APPROVED: return 'green'; default: return null; } };

This leads to a mismatch between the badge colors and the submission bar color. SUBMITTED and IN_REVIEW have the same status bar text, so I figured they should have the same color as well.

I assumed APPROVED should be green, but maybe we don't need a status bar when a boundary is approved.

maybe we don't need a status bar when a boundary is approved.

Correct on the green. There is a bar for approved boundaries, shown here:

mstone121 · 2022-10-24T17:18:56Z

c9d657d - Validator Approve button

Should administrator also see this?

rajadain · 2022-10-24T17:32:03Z

Should administrator also see this?

Yes

mstone121 · 2022-10-24T18:08:43Z

Also regarding the "Mark approved" button: I couldn't find an empty circle hero icon so I used the checked circle instead.

rajadain · 2022-10-24T18:20:36Z

Going to take another look

rajadain

+1 tested, nice work capturing all the different moving parts to this. I made #151 for handling the actual approval process.

A couple last minor comments to get this working, then this should be good to merge.

rajadain · 2022-10-24T18:33:27Z

src/app/src/components/Submissions/Detail/Map.js

+            case BOUNDARY_STATUS.SUBMITTED:
+            case BOUNDARY_STATUS.IN_REVIEW:
+                return 'Your map will be reviewed.';


Suggested change

case BOUNDARY_STATUS.SUBMITTED:

case BOUNDARY_STATUS.IN_REVIEW:

return 'Your map will be reviewed.';

case BOUNDARY_STATUS.SUBMITTED:

return 'Your map will be reviewed.';

case BOUNDARY_STATUS.IN_REVIEW:

return 'Your map is being reviewed.';

rajadain · 2022-10-24T18:36:30Z

src/app/src/components/Submissions/Detail/Map.js

+    };
+
+    const getReviewDetailText = review => {
+        const commentCount = review.comments.length;


Suggested change

const commentCount = review.comments.length;

const commentCount = review.annotations.length;

Other files will use this soon.

Having a single date format will make the app feel more consistent.

Making the SubmissionStatusBar a hook allows its return value to be checked for null. If it were a component, the parent (in this case Map), would need to determine whether or not it should render. This would involve repeating much of the logic that already exists in theS SubmissionStatusBar.

mstone121 force-pushed the ms/wire-up-boundary-details-page branch from fbdc272 to 9c51cec Compare October 19, 2022 21:31

mstone121 force-pushed the ms/wire-up-boundary-details-page branch from 03d821e to cd0635a Compare October 20, 2022 18:06

mstone121 requested a review from rajadain October 20, 2022 18:07

mstone121 assigned rajadain Oct 20, 2022

rajadain requested changes Oct 21, 2022

View reviewed changes

jacobtylerwalls mentioned this pull request Oct 21, 2022

Implement boundary submission form #147

Merged

8 tasks

mstone121 commented Oct 21, 2022

View reviewed changes

rajadain approved these changes Oct 24, 2022

View reviewed changes

rajadain reviewed Oct 24, 2022

View reviewed changes

src/app/src/components/Submissions/Detail/Map.js Outdated Show resolved Hide resolved

rajadain requested changes Oct 24, 2022

View reviewed changes

mstone121 commented Oct 24, 2022

View reviewed changes

rajadain approved these changes Oct 24, 2022

View reviewed changes

Matt Stone added 8 commits October 24, 2022 14:12

Add additional columns for boundary details page

7342aab

Move POLYGON_LAYER_OPTIONS to constants.py

519ed35

Other files will use this soon.

Add option to fit layer to useMapLayer

7c8bfc3

Add formateDateTime function

a072df2

Having a single date format will make the app feel more consistent.

Add CenteredSpinner

e24037e

Add downloadData utility function

795e7a7

Wire up boundary details page

a2f625b

Update CHANGELOG.md

b1cf7af

Matt Stone added 3 commits October 24, 2022 14:15

Add start review view/serializers/endpoint

26c3b3f

Remove fixed height badge styles

445261e

mstone121 force-pushed the ms/wire-up-boundary-details-page branch from d5af311 to 2a1dbf8 Compare October 24, 2022 19:16

mstone121 merged commit c087c57 into develop Oct 24, 2022

mstone121 deleted the ms/wire-up-boundary-details-page branch October 24, 2022 19:21

jacobtylerwalls mentioned this pull request Oct 24, 2022

Add missing migration for User.full_name #152

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Wire up boundary details page #143

Wire up boundary details page #143

mstone121 commented Oct 19, 2022 •

edited by rajadain

Loading

rajadain commented Oct 20, 2022

mstone121 commented Oct 20, 2022

rajadain commented Oct 20, 2022

rajadain left a comment

mstone121 left a comment

rajadain left a comment

rajadain commented Oct 24, 2022

rajadain commented Oct 24, 2022

rajadain left a comment •

edited

Loading

mstone121 Oct 24, 2022

mstone121 Oct 24, 2022

rajadain Oct 24, 2022

mstone121 commented Oct 24, 2022

rajadain commented Oct 24, 2022

mstone121 commented Oct 24, 2022

rajadain commented Oct 24, 2022

rajadain left a comment

rajadain Oct 24, 2022

rajadain Oct 24, 2022

	const commentCount = review.comments.length;
	const commentCount = review.annotations.length;

Wire up boundary details page #143

Wire up boundary details page #143

Conversation

mstone121 commented Oct 19, 2022 • edited by rajadain Loading

Overview

Notes

Testing Instructions

Checklist

rajadain commented Oct 20, 2022

mstone121 commented Oct 20, 2022

rajadain commented Oct 20, 2022

rajadain left a comment

Choose a reason for hiding this comment

mstone121 left a comment

Choose a reason for hiding this comment

rajadain left a comment

Choose a reason for hiding this comment

rajadain commented Oct 24, 2022

rajadain commented Oct 24, 2022

rajadain left a comment • edited Loading

Choose a reason for hiding this comment

mstone121 Oct 24, 2022

Choose a reason for hiding this comment

mstone121 Oct 24, 2022

Choose a reason for hiding this comment

rajadain Oct 24, 2022

Choose a reason for hiding this comment

mstone121 commented Oct 24, 2022

rajadain commented Oct 24, 2022

mstone121 commented Oct 24, 2022

rajadain commented Oct 24, 2022

rajadain left a comment

Choose a reason for hiding this comment

rajadain Oct 24, 2022

Choose a reason for hiding this comment

rajadain Oct 24, 2022

Choose a reason for hiding this comment

mstone121 commented Oct 19, 2022 •

edited by rajadain

Loading

rajadain left a comment •

edited

Loading