adding step letters to abl #1953

SidelineCory24 · 2022-02-01T14:29:43Z

BE PR

To-Do & Feedback:

mjoyce91 · 2022-02-08T14:25:15Z

@SidelineCory24 - is this ready for review?

SidelineCory24 · 2022-02-08T14:26:32Z

@SidelineCory24 - is this ready for review?

Yes. I see I only tagged someone on the BE PR 😅

SidelineCory24 · 2022-02-08T14:27:26Z

@mjoyce91 I'm having the code climate/coverage issue again and there's no info when I click the details.

mjoyce91 · 2022-02-09T16:00:22Z

@SidelineCory24 - just need to bump your test coverage

mjoyce91 · 2022-02-09T16:21:37Z

Looks like the Bureau Share lock icon got thrown off to the right:

mjoyce91 · 2022-02-09T16:22:33Z

Date picker isn't displaying correctly:

mjoyce91 · 2022-02-09T16:24:44Z

src/Components/AvailableBidder/EditBidder/EditBidder.jsx

+  const stepLetterOneDate = get(details, 'stepLetterOne') === null ? new Date() : new Date(get(details, 'stepLetterOne'));
+  const stepLetterTwoDate = get(details, 'stepLetterTwo') === null ? new Date() : new Date(get(details, 'stepLetterTwo'));


These should probably default to null instead of new Date()? Otherwise it auto-fills the input field with a date, even if the person didn't originally have step letter dates.

Defaulting to current date:

mjoyce91 · 2022-02-09T16:28:35Z

Optional suggestion: some users may not know that they can just delete the date to clear it. Maybe a red X icon like we have elsewhere?

SidelineCory24 · 2022-02-09T16:44:43Z

Optional suggestion: some users may not know that they can just delete the date to clear it. Maybe a red X icon like we have elsewhere?

Good idea!

SidelineCory24 · 2022-02-09T16:49:27Z

@mjoyce91 in reference to the styling fixes, should I do that on this PR or the cleanup ticket that I'm working on with the other styling updates?

mjoyce91 · 2022-02-09T16:57:45Z

@mjoyce91 in reference to the styling fixes, should I do that on this PR or the cleanup ticket that I'm working on with the other styling updates?

I would do it in this one. I think of the cleanup more specifically as how we display the data in the table

mjoyce91 · 2022-02-09T16:59:46Z

~~Receiving an error when I try to submit data~~

@SidelineCory24 - this actually isn't because of anything new in your code. This happens when you try to update a bidder with a null status.

…lting/State-TalentMap into feature/abl-step-letters

mjoyce91 · 2022-02-09T21:05:10Z

src/Components/AvailableBidder/AvailableBidderRow/AvailableBidderRow.jsx

+  const stepLetterOne = get(bidder, 'available_bidder_details.step_letter_one');
+  const formattedStepLetterOne = stepLetterOne ? formatDate(stepLetterOne) : NO_DATE;
+  const stepLetterTwo = get(bidder, 'available_bidder_details.step_letter_two');
+  const formattedStepLetterTwo = stepLetterTwo ? formatDate(stepLetterTwo) : NO_DATE;


What if we used "D/M/YY" instead of "DD/MM/YYYY" to save space?

Ex: 02/08/2022 becomes 2/8/22

replaced with tooltip/icon

mjoyce91 · 2022-02-09T21:06:50Z

src/Components/AvailableBidder/AvailableBidderRow/AvailableBidderRow.jsx

+  const stepLetterOne = get(bidder, 'available_bidder_details.step_letter_one');
+  const formattedStepLetterOne = stepLetterOne ? formatDate(stepLetterOne) : NO_DATE;
+  const stepLetterTwo = get(bidder, 'available_bidder_details.step_letter_two');
+  const formattedStepLetterTwo = stepLetterTwo ? formatDate(stepLetterTwo) : NO_DATE;


Do we need to show "None listed" for the second if there's only one step letter?

I was wondering that myself. I can remove it and it back if more clarity is needed. They can always use the editor as well to see the second letter has no date.

ss in above comment to show update

mjoyce91 · 2022-02-09T21:07:44Z

src/Components/AvailableBidder/EditBidder/EditBidder.jsx

+        <div>
+          <dt>Step Letter 1:</dt>
+          <DatePicker
+            selected={stepLetterOne}
+            onChange={d => { updateStepLetterOne(d); }}
+            dateFormat="MMMM d, yyyy"
+          />
+        </div>
+        <div>
+          <dt>Step Letter 2:</dt>
+          <DatePicker
+            selected={stepLetterTwo}
+            onChange={d => { updateStepLetterTwo(d); }}
+            dateFormat="MMMM d, yyyy"
+          />
+        </div>


Question: should we do data validation to ensure that Step Letter 1 exists before allowing them to enter Step Letter 2?

And then it goes in reverse as well. If Step Letter 1 and Step Letter 2 both exist, can they delete Step Letter 1 without deleting Step Letter 2?

Data validation makes sense. I'll do the None listed styling like the OC edit options.

For the reverse option, making step1 read only if step2 isn't null sounds good since step1 has to exist before step2.

SidelineCory24 · 2022-02-15T17:07:53Z

@mjoyce91 larger date picker styling has been pushed

mjoyce91 · 2022-02-15T17:45:43Z

These should get asterisk added to designate as CDO-only

mjoyce91 · 2022-02-15T19:24:59Z

One edge case that isn't accounted for:

Step 1. Give an available bidder 2 step letters
Step 2. Re-open the edit modal for that bidder
Step 3. Manually highlight and delete the date text for Step Letter 1. Step Letter 2 will show "None listed" but if you save, then Step Letter 2 date is still send in request payload.

* dev: abl table cleanup (#1976) # Conflicts: # src/Components/AvailableBidder/AvailableBidderRow/AvailableBidderRow.jsx # src/sass/_availableBidders.scss

SidelineCory24 · 2022-02-15T21:46:07Z

@elizabeth-jimenez @mjoyce91 this should show all the changes based on feedback.

step_letter_feedback.mp4

Step letter one has been removed manually and Step letter two still exist in the payload

Step letter one date > Step letter two date

mjoyce91 · 2022-02-15T23:10:07Z

@SidelineCory24 I like the SL2 before SL1 date check. There may be a way to add that validation to the datepicker itself so that you can't even click on a past date. That can be a tech debt task.

SidelineCory24 · 2022-02-15T23:55:18Z

@SidelineCory24 I like the SL2 before SL1 date check. There may be a way to add that validation to the datepicker itself so that you can't even click on a past date. That can be a tech debt task.

done
new icon for step letters is shown as well

step_letter_date_disabled.mp4

elizabeth-jimenez · 2022-02-16T13:19:44Z

so nice having the previous dates un-selectable!

elizabeth-jimenez · 2022-02-16T13:24:53Z

One edge case that isn't accounted for:

Step 1. Give an available bidder 2 step letters Step 2. Re-open the edit modal for that bidder Step 3. Manually highlight and delete the date text for Step Letter 1. Step Letter 2 will show "None listed" but if you save, then Step Letter 2 date is still send in request payload.

this problem doesn't exist anymore cause the user is prevented from submitting - but when they re-add their SL1, the old SL2 pops back in where None Listed is, not sure if we want to address that or not

elizabeth-jimenez · 2022-02-16T13:27:02Z

src/Components/AvailableBidder/EditBidder/EditBidder.jsx

+          {stepLetterTwoFlag &&
+            <div className="step-letter-clear-icon">
+              <FA name="times-circle fa-lg inactive" />
+            </div>
+          }
+          {!stepLetterTwoFlag &&
+            <div className="step-letter-clear-icon">
+              <InteractiveElement
+                onClick={clearStepLetterTwoDate}
+              >
+                <FA name="times-circle fa-lg active" />
+              </InteractiveElement>
+            </div>
+          }


can this be a .... 🥁 ternary?

Yeah really lines 234-264 could all be encapsulated in a single ternary

ternary fix has been pushed

can they mike? like one big one? I see two smaller ones but can't see the big one :(

mjoyce91 · 2022-02-16T13:41:49Z

One edge case that isn't accounted for:
Step 1. Give an available bidder 2 step letters Step 2. Re-open the edit modal for that bidder Step 3. Manually highlight and delete the date text for Step Letter 1. Step Letter 2 will show "None listed" but if you save, then Step Letter 2 date is still send in request payload.

this problem doesn't exist anymore cause the user is prevented from submitting - but when they re-add their SL1, the old SL2 pops back in where None Listed is, not sure if we want to address that or not

@elizabeth-jimenez - I think I like current functionality. That way if they delete SL1 on accident, they don't lose the SL2 date.

elizabeth-jimenez · 2022-02-16T13:44:05Z

One edge case that isn't accounted for:
Step 1. Give an available bidder 2 step letters Step 2. Re-open the edit modal for that bidder Step 3. Manually highlight and delete the date text for Step Letter 1. Step Letter 2 will show "None listed" but if you save, then Step Letter 2 date is still send in request payload.

this problem doesn't exist anymore cause the user is prevented from submitting - but when they re-add their SL1, the old SL2 pops back in where None Listed is, not sure if we want to address that or not

@elizabeth-jimenez - I think I like current functionality. That way if they delete SL1 on accident, they don't lose the SL2 date.

ok cool 😁 - I could see the pros and cons to it

elizabeth-jimenez · 2022-02-16T13:46:33Z

src/sass/_availableBidders.scss

@@ -407,6 +460,7 @@
 }

 .tippy-popper {
+  // TODO - consolidate these two classes


i'm looking into consolidating them, but all good if we dont!

elizabeth-jimenez

i win

elizabeth-jimenez · 2022-02-16T14:05:06Z

I got the consolidation!

.tippy-popper {
  .tippy-tooltip.ab-row-theme, .tippy-tooltip.step-letters-theme {
    background-color: $color-white;
    color: $color-black;
    border: 2px solid $tm-gray;
    font-size: 14px;
    font-family: Roboto;
    padding: 15px;
    text-align: left;
    box-shadow: $tm-gray 3px 14px 29px;
    .arrow-regular {
      border: none;
    }
    .ab-row-tooltip-wrapper, .step-letter-tooltip-wrapper {
      display: flex;
      padding-bottom: 10px;
      div {
        display: flex;
        flex-direction: column;
        align-items: flex-start;
      }
      .title {
        font-size: 16px;
        font-weight: bold;
      }
      .ab-row-tooltip-data, .step-letter-date {
        font-weight: 400;
        font-style: italic;
      }
    }

    .step-letter-tooltip-wrapper {
      div:first-child {
        padding-right: 10px;
        border-right: 1px solid $color-gray-light;
      }
      div:nth-child(2) {
        padding-left: 10px;
      }
    }

    .title {
      font-size: 16px;
      font-weight: bold;
    }
  }
}

SidelineCory24 · 2022-02-16T15:08:00Z

I got the consolidation!

.tippy-popper {
  .tippy-tooltip.ab-row-theme, .tippy-tooltip.step-letters-theme {
    background-color: $color-white;
    color: $color-black;
    border: 2px solid $tm-gray;
    font-size: 14px;
    font-family: Roboto;
    padding: 15px;
    text-align: left;
    box-shadow: $tm-gray 3px 14px 29px;
    .arrow-regular {
      border: none;
    }
    .ab-row-tooltip-wrapper, .step-letter-tooltip-wrapper {
      display: flex;
      padding-bottom: 10px;
      div {
        display: flex;
        flex-direction: column;
        align-items: flex-start;
      }
      .title {
        font-size: 16px;
        font-weight: bold;
      }
      .ab-row-tooltip-data, .step-letter-date {
        font-weight: 400;
        font-style: italic;
      }
    }

    .step-letter-tooltip-wrapper {
      div:first-child {
        padding-right: 10px;
        border-right: 1px solid $color-gray-light;
      }
      div:nth-child(2) {
        padding-left: 10px;
      }
    }

    .title {
      font-size: 16px;
      font-weight: bold;
    }
  }
}

this consolidation is being done on another pr

adding step letters to abl

fd0525f

SidelineCory24 added do not merge WIP Work in progress dual merge labels Feb 1, 2022

Merge branch 'dev' into feature/abl-step-letters

646c051

SidelineCory24 mentioned this pull request Feb 3, 2022

adding step letters to abl MetaPhase-Consulting/State-TalentMAP-API#663

Merged

4 tasks

hooking to BE

5cc9341

SidelineCory24 removed do not merge WIP Work in progress labels Feb 7, 2022

Merge branch 'dev' into feature/abl-step-letters

d42f6ba

SidelineCory24 requested a review from mjoyce91 February 8, 2022 14:26

Merge branch 'dev' into feature/abl-step-letters

629930c

mjoyce91 reviewed Feb 9, 2022

View reviewed changes

SidelineCory24 added 3 commits February 9, 2022 12:59

aligning bureau icon in editor

8493934

Merge branch 'feature/abl-step-letters' of github.com:MetaPhase-Consu…

dd8b979

…lting/State-TalentMap into feature/abl-step-letters

updating default date to null

8163476

mjoyce91 reviewed Feb 9, 2022

View reviewed changes

fixing clear date functionality

94400ba

SidelineCory24 and others added 3 commits February 15, 2022 15:53

styling & error handling for editor

e29a4b0

error handling if step1date is greater than step2date

6bcb75f

Merge branch 'dev' into feature/abl-step-letters

3f8439d

* dev: abl table cleanup (#1976) # Conflicts: # src/Components/AvailableBidder/AvailableBidderRow/AvailableBidderRow.jsx # src/sass/_availableBidders.scss

changing step letters icon/coloring

5eb85be

SidelineCory24 added 2 commits February 15, 2022 18:27

snapshot/test

95820b0

disabling dates in calendar when sl1 is greather than sl2

f03e693

SidelineCory24 added 2 commits February 15, 2022 18:58

snapshot updates

dfd4d8c

snapshot updates/calendar update

bed3d1c

elizabeth-jimenez reviewed Feb 16, 2022

View reviewed changes

SidelineCory24 added 2 commits February 16, 2022 08:49

ternary for step2 clear icon

edad70d

ternary for step2 date picker

7994cf6

elizabeth-jimenez approved these changes Feb 16, 2022

View reviewed changes

mjoyce91 approved these changes Feb 16, 2022

View reviewed changes

mjoyce91 merged commit f65f84e into dev Feb 16, 2022

mjoyce91 deleted the feature/abl-step-letters branch February 16, 2022 14:07

SidelineCory24 mentioned this pull request Feb 16, 2022

abl tooltip styling #1978

Merged

		const stepLetterOneDate = get(details, 'stepLetterOne') === null ? new Date() : new Date(get(details, 'stepLetterOne'));
		const stepLetterTwoDate = get(details, 'stepLetterTwo') === null ? new Date() : new Date(get(details, 'stepLetterTwo'));

adding step letters to abl #1953

adding step letters to abl #1953

Conversation

SidelineCory24 commented Feb 1, 2022 • edited Loading

mjoyce91 commented Feb 8, 2022

SidelineCory24 commented Feb 8, 2022

SidelineCory24 commented Feb 8, 2022

mjoyce91 commented Feb 9, 2022

mjoyce91 commented Feb 9, 2022

mjoyce91 commented Feb 9, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mjoyce91 commented Feb 9, 2022

SidelineCory24 commented Feb 9, 2022

SidelineCory24 commented Feb 9, 2022

mjoyce91 commented Feb 9, 2022

mjoyce91 commented Feb 9, 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mjoyce91 Feb 9, 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

SidelineCory24 commented Feb 15, 2022

mjoyce91 commented Feb 15, 2022

mjoyce91 commented Feb 15, 2022 • edited Loading

SidelineCory24 commented Feb 15, 2022 • edited Loading

mjoyce91 commented Feb 15, 2022

SidelineCory24 commented Feb 15, 2022 • edited Loading

elizabeth-jimenez commented Feb 16, 2022

elizabeth-jimenez commented Feb 16, 2022

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

mjoyce91 commented Feb 16, 2022

elizabeth-jimenez commented Feb 16, 2022

Choose a reason for hiding this comment

elizabeth-jimenez left a comment

Choose a reason for hiding this comment

elizabeth-jimenez commented Feb 16, 2022

SidelineCory24 commented Feb 16, 2022

SidelineCory24 commented Feb 1, 2022 •

edited

Loading

mjoyce91 commented Feb 9, 2022 •

edited

Loading

mjoyce91 Feb 9, 2022 •

edited

Loading

mjoyce91 commented Feb 15, 2022 •

edited

Loading

SidelineCory24 commented Feb 15, 2022 •

edited

Loading

SidelineCory24 commented Feb 15, 2022 •

edited

Loading