keyword search for quantifiers & Quantify multiple praise #499

nebs-dev · 2022-06-23T15:28:07Z

Resolves #422

Added search by praise reason
Added option so quantify multiple praise at once

mattyg

Awesome this looks great!

I requested some changes inline.

Also a few additional I'd like to request:

After dismissal, quantifying multiple, or marking duplicates, the "Select All" checkbox itself is unchecked
Write tests for the new quantifyMultiple controller (you can do this in another PR -- you'll mostly copy the praise controller tests from pr Test: additional api tests #479)

mattyg · 2022-06-26T16:55:12Z

packages/frontend/src/pages/QuantifyPeriodReceiver/components/QuantifyTable.tsx


  const allowedValues = usePeriodSettingValueRealized(
    periodId,
    'PRAISE_QUANTIFY_ALLOWED_VALUES'
  ) as number[];

+  const applyFilter = React.useCallback(


Should we rename this function to be a bit semantically clearer? Maybe filterBySearchValue or something?

mattyg · 2022-06-26T16:55:42Z

packages/frontend/src/pages/QuantifyPeriodReceiver/components/QuantifyTable.tsx

+      if (!searchValue) return data;
+
+      const filteredData = data.filter((praise: PraiseDto) => {
+        return praise.reasonRealized.includes(searchValue);


Do you think we should filter by both reasonRealized and user name?

and which username should we use? receiver's or giver's?

how about both?

mattyg · 2022-06-26T16:56:17Z

packages/frontend/src/pages/QuantifyPeriodReceiver/components/QuantifyTable.tsx

+      if (!searchValue) return data;
+
+      const filteredData = data.filter((praise: PraiseDto) => {
+        return praise.reasonRealized.includes(searchValue);


Should we make the search case-insensitive?

mattyg · 2022-06-26T17:02:42Z

packages/frontend/src/pages/QuantifyPeriodReceiver/components/QuantifyTable.tsx

+              <td colSpan={5}>
+                <div className="mb-5 border-2 border-t border-warm-gray-400" />
+              </td>
+            </tr>


What do you think about moving the 'Select All' checkbox into the "toolbar" area above? That way it will be "sticky" when user scrolls down the page. Also then we could remove this additional border line.

Something like this:

mattyg · 2022-06-26T17:06:01Z

packages/frontend/src/pages/QuantifyPeriodReceiver/components/QuantifyMultipleButton.tsx

+      }
+      onClick={onClick}
+    >
+      <FontAwesomeIcon icon={faPrayingHands} size="1x" />


What do you think about using the "unbalanced scale" icon instead of praying hands? https://fontawesome.com/search?q=scale%20unbalanced&s=solid%2Cbrands%2Cregular

packages/frontend/src/pages/QuantifyPeriodReceiver/components/QuantifyMultipleDialog.tsx

mattyg · 2022-06-26T17:07:41Z

packages/frontend/src/pages/EventLogs/EventLogsPage.tsx

@@ -109,7 +109,7 @@ const EventLogsPage = (): JSX.Element => {
          </div>

          {/* Search */}
-          <div className="w-5/12 mt-5 mb-5 mr-4">
+          <div className="w-5/12 mt-5 mb-5 mr-4 h-[42px]">


Can we use tailwind's built-in sizing classes instead of fixed pixel sizes?

We just want to ensure all the inputs are the same height. If that can be achieved without specifying a fixed height, all the better.

mattyg · 2022-06-26T17:07:48Z

packages/frontend/src/pages/EventLogs/EventLogsPage.tsx

@@ -120,7 +120,7 @@ const EventLogsPage = (): JSX.Element => {
          </div>

          {/* Sort */}
-          <div className="w-2/12 mt-5 mb-5 ml-auto mr-5">
+          <div className="w-2/12 mt-5 mb-5 ml-auto mr-5 h-[42px]">


same as above

mattyg · 2022-06-26T17:08:13Z

packages/frontend/src/components/form/SearchInput.tsx

@@ -25,7 +27,7 @@ const SearchInput = ({
        className="block pl-8 bg-transparent border-none outline-none focus:ring-0"
        name="search"
        type="text"
-        placeholder="Search"
+        placeholder={placeholder}


mattyg · 2022-06-26T17:12:04Z

packages/api/src/praise/controllers.ts

@@ -201,3 +202,49 @@ export const quantify = async (
  const response = await praiseDocumentListTransformer(affectedPraises);
  res.status(200).json(response);
 };
+
+export const quantifyMultiple = async (


This function will need to have same logic used in the quantify function to determine all the praises with an affected scoreRealized. It probably makes sense to pull that logic out into a utility function and use it in both controllers.

The reason for that logic is so the "Duplicate Score" display is updated when the original praise's score is updated. See the screencast below:

Kazam_screencast_00003.mp4

This function will need to have same logic used in the quantify function to determine all the praises with an affected scoreRealized. It probably makes sense to pull that logic out into a utility function and use it in both controllers.

The reason for that logic is so the "Duplicate Score" display is updated when the original praise's score is updated. See the screencast below:

Kazam_screencast_00003.mp4

☝️☝️ @nebs-dev, this remains to be done.

kristoferlund

I believe it is time to remove the text from the buttons.

Move "select all" checkbox to inline with buttons
Hide and show only relevant buttons. No items checked = no buttons visible.
Use the praise-button-round style
Tooltip below buttons describe function
(see mail.google.com for reference 😄)
Please note that image below is just wireframe, use exisiting styles and colours.

…modified inputs, changed buttons style

…x merge conflicts

…ommons-stack/praise into enh/quantifiers_keyword_search

kristoferlund

I added the IconRoundButton because I wanted another look than the regular button. And had to make a custom tooltip because the MUI one don't work over well buttons that are disabled.

We can talk the changes through tomorrow if you like?

Merging now.

kristoferlund · 2022-06-29T11:36:38Z

packages/api/src/praise/controllers.ts

@@ -201,3 +202,49 @@ export const quantify = async (
  const response = await praiseDocumentListTransformer(affectedPraises);
  res.status(200).json(response);
 };
+
+export const quantifyMultiple = async (


This function will need to have same logic used in the quantify function to determine all the praises with an affected scoreRealized. It probably makes sense to pull that logic out into a utility function and use it in both controllers.

The reason for that logic is so the "Duplicate Score" display is updated when the original praise's score is updated. See the screencast below:

Kazam_screencast_00003.mp4

☝️☝️ @nebs-dev, this remains to be done.

kristoferlund · 2022-06-29T11:42:43Z

packages/frontend/src/pages/QuantifyPeriodReceiver/components/MarkDismissedButton.tsx

@@ -3,25 +3,26 @@ import { faMinusCircle } from '@fortawesome/free-solid-svg-icons';

 interface Props {
  disabled?: boolean;
+  small?: boolean;
  onClick();
 }

 const MarkDismissedButton = ({


I'd suggest creating a more generic component, similar to IconButton. IconButtonRound perhaps?

changes made

nebs-dev added 2 commits June 23, 2022 17:22

Added option to quantify multiple praises at once

937b648

Change multiple quantify button & component name

086e226

nebs-dev requested review from mattyg and kristoferlund June 23, 2022 15:28

nebs-dev self-assigned this Jun 23, 2022

mattyg previously requested changes Jun 26, 2022

View reviewed changes

kristoferlund requested changes Jun 27, 2022

View reviewed changes

nebs-dev and others added 5 commits June 28, 2022 23:15

Moved select all checkbox, fixed issue with duplicate praise scores, …

ac0ff3f

…modified inputs, changed buttons style

Merge branch 'main' into enh/quantifiers_keyword_search

6ad48cb

Fix: Added InternalServerError import

da5b6d6

move quantify logic from controller to utils, modify icon buttons, fi…

7e27097

…x merge conflicts

Merge branch 'enh/quantifiers_keyword_search' of https://github.com/c…

013ebff

…ommons-stack/praise into enh/quantifiers_keyword_search

nebs-dev requested review from mattyg and kristoferlund June 29, 2022 18:19

kristoferlund added 4 commits June 30, 2022 17:40

Fix: Margins and colours

7acf2a1

Add: IconRoundButton

316f94a

Fix: removed round button code from IconButton

4cdb995

Fix: Remove unused imports

e697acd

kristoferlund approved these changes Jun 30, 2022

View reviewed changes

kristoferlund merged commit 9a09159 into main Jun 30, 2022

kristoferlund deleted the enh/quantifiers_keyword_search branch June 30, 2022 16:00

kristoferlund mentioned this pull request Jul 1, 2022

v0.10.0 #512

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

keyword search for quantifiers & Quantify multiple praise #499

keyword search for quantifiers & Quantify multiple praise #499

nebs-dev commented Jun 23, 2022

mattyg left a comment

mattyg Jun 26, 2022

mattyg Jun 26, 2022

nebs-dev Jun 27, 2022

mattyg Jun 27, 2022

kristoferlund Jun 27, 2022

mattyg Jun 26, 2022

kristoferlund Jun 27, 2022

mattyg Jun 26, 2022

mattyg Jun 26, 2022

mattyg Jun 26, 2022

kristoferlund Jun 27, 2022

mattyg Jun 26, 2022

mattyg Jun 26, 2022

mattyg Jun 26, 2022 •

edited

Loading

kristoferlund Jun 29, 2022

kristoferlund left a comment

kristoferlund left a comment

kristoferlund Jun 29, 2022

kristoferlund Jun 29, 2022

keyword search for quantifiers & Quantify multiple praise #499

keyword search for quantifiers & Quantify multiple praise #499

Conversation

nebs-dev commented Jun 23, 2022

mattyg 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

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

Choose a reason for hiding this comment

mattyg Jun 26, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kristoferlund left a comment

Choose a reason for hiding this comment

kristoferlund left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mattyg Jun 26, 2022 •

edited

Loading