LF-4371 Feedback - Add feedback drawer and icon #3532

kathyavini · 2024-11-18T18:23:09Z

Description

Thank you @kevin-wu01 for doing the bulk of this ticket, including the integration into App! 🙏

This ticket covers the side drawer component and the menu icon that calls it up. I did throw the existing feedback form (with no alternations) into the Drawer, but there is a separate ticket for the feedback form refactor, including removing the old route and adjusting the form content, that I will open separately.

Notes:

The Jira ticket called for SideDrawer to be its own component that is used in Desktop view, and the existing Drawer to be used in mobile view; however in the end I made SideDrawer a variant of Drawer to
- keep the form content from being cleared upon resize across the breakpoint
- reduce code duplication between Drawer and SideDrawer
- not repeat the mobile calculations in both the container and the Drawer itself
I couldn't locate a working Storybook story for the existing Drawer (there is one for NavBarDrawer but it doesn't open for me and it's not exactly for this component itself).
- Here is the new Storybook story for Drawer, which also hopefully makes the variant organization clear

Jira link: https://lite-farm.atlassian.net/browse/LF-4371

Type of change

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update

How Has This Been Tested?

Passes test case
UI components visually reviewed on desktop view
UI components visually reviewed on mobile view
Other (please explain)

Checklist:

I have commented my code, particularly in hard-to-understand areas
My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally with my changes
The precommit and linting ran successfully
I have added or updated language tags for text that's part of the UI
I have added "MISSING" for all new language tags to languages I don't speak
I have added the GNU General Public License to all new files

…ggering type error

…); apply hover state + animation + mobile width

…r Jira ticket instructions); remove mobile styling from SideDrawer

…e mobile slide-up animation The offending line was even mentioned in the PR discussion thread, but I didn't realize at the time the return null ruins the ability to animate between the open and closed states! (Luckily bug never made it to App)

Also minor fixes: remove classname with no reference in JSX and remove propTypes which were triggering type error

… Drawer

kathyavini · 2024-11-18T18:30:42Z

packages/webapp/src/components/Navigation/SideMenu/index.jsx

@@ -233,7 +233,7 @@ const PureSideMenu = ({
        isOpen={isDrawerOpen}
        onClose={onDrawerClose}
        fullHeight
-        responsiveModal={false}
+        desktopVariant={DesktopDrawerVariants.DRAWER}


@antsgar I don't know if you remember back well enough to when the responsiveModal property was defined, but it doesn't seem necessary in this context because the Drawer is never a modal in mobile view anyway! Maybe Drawer or SideMenu also got a refactor since originally written?

Yeah since this only renders in mobile view I don't think the prop makes sense, I'm not sure why I passed it along initially or if something got refactored along the way

kathyavini · 2024-11-18T18:33:11Z

packages/webapp/src/components/Drawer/index.tsx

-  }
-
-  return isDesktop && responsiveModal ? (
+  return isDesktop && desktopVariant === DesktopDrawerVariants.MODAL && isOpen ? (


I actually destroyed the mobile drawer slide-up animation with this change, which we did discuss a little in this PR comment thread but I landed on thinking it was innocuous. At the time I totally missed that applying conditional rendering to the Drawer = no animation! Luckily it never went to App, but you can see the slide up animation broken on beta/integration currently.

Oooh great catch!!

SayakaOno

Looks great to me! Nice stories ❤️

antsgar

Looks awesome! ☺️

I found a few visual differences with the designs, for example:

The padding between the form and the drawer borders should be narrower (the drawer itself already has padding so those 16px are summed to the 24px of the container here, but it should be 24px total).
The "Request help" title is not present in the design.
There's wider spacing between some form fields, like the radio buttons and the email field.
The buttons should be anchored to the bottom of the drawer if possible per the designs.

Not sure if I'm looking at the right design though! But other than these it looks great.

antsgar · 2024-11-20T15:21:55Z

packages/webapp/src/components/Drawer/index.tsx

-  }
-
-  return isDesktop && responsiveModal ? (
+  return isDesktop && desktopVariant === DesktopDrawerVariants.MODAL && isOpen ? (


Oooh great catch!!

antsgar · 2024-11-20T15:23:52Z

packages/webapp/src/components/Navigation/SideMenu/index.jsx

@@ -233,7 +233,7 @@ const PureSideMenu = ({
        isOpen={isDrawerOpen}
        onClose={onDrawerClose}
        fullHeight
-        responsiveModal={false}
+        desktopVariant={DesktopDrawerVariants.DRAWER}


Yeah since this only renders in mobile view I don't think the prop makes sense, I'm not sure why I passed it along initially or if something got refactored along the way

kathyavini · 2024-11-20T16:30:43Z

@antsgar the styling of the form had its own ticket in Jira and the PR for that is open here:

LF-4372 Feedback - move feedback form and delete old routes #3535

(It's only in draft because it's branched off of / depending on this one, but actually I'll undraft it because the code is done). Could you check the points you mentioned against that PR branch?

antsgar · 2024-11-21T21:21:00Z

@kathyavini ahhh that makes sense, thanks! I'll approve this one and review the other one

Duncan-Brain · 2024-11-22T00:07:31Z

@kathyavini Any reason you can think of why the Happy path tests might be failing haha?

kathyavini · 2024-11-22T00:23:47Z

@Duncan-Brain I see that same failure locally 🤔

Could it be that cypress is still reading the elements of the off-viewport form (closed state of sideDrawer is off viewport to the right)? The selector that is failing is suuuuuuper general. Too general I would say 😂 Since it's just the one written like this, I'll probably make it more selective in the test.

    cy.get(Selectors.REACT_SELECT)
      .find('input')

Duncan-Brain · 2024-11-22T00:30:42Z

@kathyavini That makes sense to me if it is being added to both forms yah! ps I am impressed you can run happy path locally -- I don't want to even try haha

kevin-wu01 and others added 11 commits September 17, 2024 14:21

LF-4371 add icon button

80c5949

LF-4371 push changes to branch

3db0555

Merge branch 'integration' into LF-4371-add-feedback

6fa4eac

LF-4371 Add licenses to new files

a8d37d9

LF-4371 Add Storybook for SideDrawer; remove propTypes which were tri…

4573b6f

…ggering type error

LF-4371 Import the send icon from Figma (slightly rounder than FiSend…

a255a6e

…); apply hover state + animation + mobile width

LF-4371 Add toggle between Drawer and SideDrawer to Feedback form (pe…

f3f874e

…r Jira ticket instructions); remove mobile styling from SideDrawer

LF-4371 Create a Storybook story for Drawer

d79c81c

Also minor fixes: remove classname with no reference in JSX and remove propTypes which were triggering type error

LF-4371 Remove SideDrawer component in favour of making it variant of…

47e2bd4

… Drawer

LF-4371 Add translation string

ae13ddb

kathyavini added the enhancement New feature or request label Nov 18, 2024

kathyavini requested review from a team as code owners November 18, 2024 18:23

kathyavini requested review from antsgar and SayakaOno and removed request for a team November 18, 2024 18:23

kathyavini self-assigned this Nov 18, 2024

kathyavini commented Nov 18, 2024

View reviewed changes

SayakaOno approved these changes Nov 18, 2024

View reviewed changes

kathyavini mentioned this pull request Nov 19, 2024

LF-4372 Feedback - move feedback form and delete old routes #3535

Merged

16 tasks

antsgar reviewed Nov 20, 2024

View reviewed changes

antsgar self-requested a review November 21, 2024 21:21

antsgar approved these changes Nov 21, 2024

View reviewed changes

antsgar added this pull request to the merge queue Nov 21, 2024

Merged via the queue into integration with commit 3eb125f Nov 21, 2024
4 of 5 checks passed

antsgar deleted the LF-4371-add-feedback branch November 21, 2024 21:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

LF-4371 Feedback - Add feedback drawer and icon #3532

LF-4371 Feedback - Add feedback drawer and icon #3532

kathyavini commented Nov 18, 2024 •

edited

Loading

kathyavini Nov 18, 2024

antsgar Nov 20, 2024

kathyavini Nov 18, 2024 •

edited

Loading

antsgar Nov 20, 2024

SayakaOno left a comment

antsgar left a comment

antsgar Nov 20, 2024

antsgar Nov 20, 2024

kathyavini commented Nov 20, 2024 •

edited

Loading

antsgar commented Nov 21, 2024

Duncan-Brain commented Nov 22, 2024

kathyavini commented Nov 22, 2024 •

edited

Loading

Duncan-Brain commented Nov 22, 2024 •

edited

Loading

LF-4371 Feedback - Add feedback drawer and icon #3532

LF-4371 Feedback - Add feedback drawer and icon #3532

Conversation

kathyavini commented Nov 18, 2024 • edited Loading

kathyavini Nov 18, 2024

Choose a reason for hiding this comment

antsgar Nov 20, 2024

Choose a reason for hiding this comment

kathyavini Nov 18, 2024 • edited Loading

Choose a reason for hiding this comment

antsgar Nov 20, 2024

Choose a reason for hiding this comment

SayakaOno left a comment

Choose a reason for hiding this comment

antsgar left a comment

Choose a reason for hiding this comment

antsgar Nov 20, 2024

Choose a reason for hiding this comment

antsgar Nov 20, 2024

Choose a reason for hiding this comment

kathyavini commented Nov 20, 2024 • edited Loading

antsgar commented Nov 21, 2024

Duncan-Brain commented Nov 22, 2024

kathyavini commented Nov 22, 2024 • edited Loading

Duncan-Brain commented Nov 22, 2024 • edited Loading

kathyavini commented Nov 18, 2024 •

edited

Loading

kathyavini Nov 18, 2024 •

edited

Loading

kathyavini commented Nov 20, 2024 •

edited

Loading

kathyavini commented Nov 22, 2024 •

edited

Loading

Duncan-Brain commented Nov 22, 2024 •

edited

Loading