Polish admin posting pages #203

sherryhli · 2022-03-10T21:07:43Z

Ticket link

Closes fullcalendar/fullcalendar-react#138

Implementation description

In addition to completing requirements outlined in the ticket, this PR also...

Adds new DateTimeUtils functions
Fixes datetime bugs
Fixes UI bugs
Sets current week in shift calendar
Executes createShifts mutation in create posting review page

Please see commit messages for more details.

Steps to test

Run the application locally (avoiding writes to staging DB for now to keep it clean)

docker-compose up --build

Go to the create posting page and create a posting
Verify data in the database

docker exec -it sistering_db/bin/bash -c "psql -U postgres -d sistering"

SELECT * FROM postings;
SELECT * FROM shifts WHERE posting_id = 'INSERT_POSTING_ID';

What should reviewers focus on?

Please review commit by commit
End-to-end functionality and correctness of behaviour
Code cleanliness
General adherence to hi-fi designs (note: another round of refinement is needed to be pixel-perfect, will require a design review)

Checklist

My PR name is descriptive and in imperative tense
My commit messages are descriptive and in imperative tense. My commits are atomic and trivial commits are squashed or fixup'd into non-trivial commits
I have run the appropriate linter(s)
I have requested a review from the PL, as well as other devs who have background knowledge on this PR or who will be building on top of this PR

github-actions · 2022-03-10T21:11:52Z

Visit the preview URL for this PR (updated for commit 51ecdcd):

https://sistering-dev--pr203-sherry-polish-admin-surs4jdg.web.app

_{(expires Thu, 31 Mar 2022 17:35:49 GMT)}

_{🔥 via Firebase Hosting GitHub Action 🌎}

sherryhli · 2022-03-14T21:14:04Z

frontend/src/components/admin/posting/CreatePostingShifts.tsx

+       * TODO: implement better solution described here
+       * https://github.com/fullcalendar/fullcalendar/issues/4684#issuecomment-620787260
+       */}
+      <div key={startDate}>


More context:
We need to set the initial date of the calendar to be in the week of the start date, which can be done via the intialDate prop passed to FullCalendar. The start date changes depending on user input on the create shifts page. However, FullCalendar does not re-render when the prop changes. This is a temp hack to force a re-render.

https://github.com/fullcalendar/fullcalendar-react/issues/57

Wait if posting start date is not on a sunday, is it supposed to rerender? Bc it does not for me.

Also, I realized if they make a posting that does not span the entire week, like wednesday to friday, the shift calendar will still allow you to create shifts for the entire week. It's kind of confusing bc when you input the dates, the format of the date doesn't say what day of the week it is.

Summary of what we discussed on Sunday:

Wait if posting start date is not on a sunday, is it supposed to rerender? Bc it does not for me.

The calendar always displays a 7-day week from Sun-Sat. A re-render is only triggered when the date of the Sunday before start date changes (e.g. the Sunday before March 22nd is March 20th, the Sunday before March 24th is also March 20th, so changing the start date from the 22nd to the 24th will not trigger a re-render)

Also, I realized if they make a posting that does not span the entire week, like wednesday to friday, the shift calendar will still allow you to create shifts for the entire week. It's kind of confusing bc when you input the dates, the format of the date doesn't say what day of the week it is.

Yep, agreed it's confusing, being addressed in #232 :)

sherryhli · 2022-03-14T21:15:11Z

frontend/src/components/common/SideNavbar.tsx

@@ -42,7 +43,8 @@ const createStepLabel = (
 const SideNavbar = (steps: SideNavbarStepsType): React.ReactElement => {
  const { activeStep, labels } = steps;
  return (
-    <Steps activeStep={activeStep} orientation="vertical">
+    // TODO: remove hardcoded width, investigate text wrapping behaviour
+    <Steps activeStep={activeStep} orientation="vertical" width="250px">


The circular number icon becomes distorted when the text spans multiple lines

This is a temporary fix and does not work for smaller screens as it affects the centering of elements within the Container

LenaNguyen

Looks really good! Thanks for making these changes :) Just a some small deviations from designs and a couple questions.

LenaNguyen · 2022-03-17T23:40:15Z

frontend/src/components/pages/admin/posting/CreatePostingBasicInfoPage.tsx

+          </Text>
+          <SideNavBar
+            activeStep={0}
+            labels={["Basic Information", "Time Slots", "Review and Post"]}


The sidenav needs the grey background to match the designs. It's #F3F3F3 in the designs so we may need to ask if that also maps to background.dark

I asked the designers it's actually background.light

LenaNguyen · 2022-03-17T23:49:03Z

frontend/src/components/admin/posting/CreatePostingShifts.tsx

+          </Button>
+          <Button onClick={handleNext}>Next</Button>
+        </HStack>
+      </VStack>


The next/back footer should be stuck to the bottom of the screen:

If we're gonna refactor so that this creation flow is all in one page, then this change is probably better there instead of this pr though!

Made a ticket to refactor into one page #213

LenaNguyen · 2022-03-17T23:57:23Z

frontend/src/components/admin/posting/CreatePostingShifts.tsx

+       * TODO: implement better solution described here
+       * https://github.com/fullcalendar/fullcalendar/issues/4684#issuecomment-620787260
+       */}
+      <div key={startDate}>


Wait if posting start date is not on a sunday, is it supposed to rerender? Bc it does not for me.

Also, I realized if they make a posting that does not span the entire week, like wednesday to friday, the shift calendar will still allow you to create shifts for the entire week. It's kind of confusing bc when you input the dates, the format of the date doesn't say what day of the week it is.

…fts form

sherryhli · 2022-03-24T17:21:52Z

Did a rebase but did not address the side navbar design deviation. It's not quite as straightforward as I thought, having trouble making the the enclosing VStack span the entire height of the container and I'll need to fiddle with the styling a bit more.

This issue, along with a couple others are documented in #233 for round 2 of refinement. @LenaNguyen if you don't mind could we go ahead and merge this PR as is?

LenaNguyen

👏👏

sherryhli force-pushed the sherry/polish-admin-posting branch from f1ee05e to 9537595 Compare March 14, 2022 05:11

sherryhli commented Mar 14, 2022

View reviewed changes

sherryhli marked this pull request as ready for review March 15, 2022 04:35

sherryhli requested review from LenaNguyen, 7tint, briantu and jiarucao March 15, 2022 04:35

LenaNguyen mentioned this pull request Mar 17, 2022

Make side navbar width dynamic #212

Closed

LenaNguyen requested changes Mar 17, 2022

View reviewed changes

sherryhli mentioned this pull request Mar 20, 2022

Implement monthly view calendar for admin scheduling UI #207

Merged

4 tasks

sherryhli added 15 commits March 22, 2022 19:11

Integrate ShiftCalendar into CreatePostingShifts

3e53c61

Use reusable FormHeader component in CreatePostingShifts

7946f0a

Navigate on 'Next' in admin posting flow

0e05dd6

Remove MainPageButton

918216c

Implement hi-fi layouts for admin posting pages

86f48b3

Add temp hack fix for SideNavbar text wrapping bug

13c21ec

Convert to UTC in DateTimeUtils functions

54be3b2

Use DateTimeUtils functions in CreatePostingReview

058f800

Adjust SideNavbar width hack

2ebe238

Create and use new DateTimeUtils

1c8934a

Set ShiftCalendar initial date based on startDate of CreatePostingShi…

b509d45

…fts form

Run createShifts mutation in CreatePostingReviewPage

f111398

Add back button to admin create posting pages

8a64dcc

Remove SideNavbarDemo page

a20d5f8

Remove Create Shifts and Review Posting buttons from default page

51ecdcd

sherryhli added the changes requested 📝 label Mar 23, 2022

sherryhli force-pushed the sherry/polish-admin-posting branch from 43a0bdc to 51ecdcd Compare March 24, 2022 17:10

sherryhli requested a review from LenaNguyen March 24, 2022 17:21

sherryhli added awaiting review 👀 and removed changes requested 📝 labels Mar 24, 2022

LenaNguyen approved these changes Mar 24, 2022

View reviewed changes

sherryhli added ship it 🚢 and removed awaiting review 👀 labels Mar 24, 2022

sherryhli merged commit beff7cf into main Mar 24, 2022

sherryhli deleted the sherry/polish-admin-posting branch March 24, 2022 22:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Polish admin posting pages #203

Polish admin posting pages #203

sherryhli commented Mar 10, 2022 •

edited

Loading

github-actions bot commented Mar 10, 2022 •

edited

Loading

sherryhli Mar 14, 2022

LenaNguyen Mar 17, 2022

sherryhli Mar 24, 2022

sherryhli Mar 14, 2022

sherryhli Mar 15, 2022

LenaNguyen left a comment

LenaNguyen Mar 17, 2022

LenaNguyen Mar 18, 2022

LenaNguyen Mar 17, 2022

LenaNguyen Mar 17, 2022

LenaNguyen Mar 17, 2022

sherryhli commented Mar 24, 2022

LenaNguyen left a comment

Polish admin posting pages #203

Polish admin posting pages #203

Conversation

sherryhli commented Mar 10, 2022 • edited Loading

Ticket link

Implementation description

Steps to test

What should reviewers focus on?

Checklist

github-actions bot commented Mar 10, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LenaNguyen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sherryhli commented Mar 24, 2022

LenaNguyen left a comment

Choose a reason for hiding this comment

sherryhli commented Mar 10, 2022 •

edited

Loading

github-actions bot commented Mar 10, 2022 •

edited

Loading