Skip to content

LF-5232 Fix farm notes modal not closing when clicking my profile icon or feedback button#4118

Merged
SayakaOno merged 6 commits intointegrationfrom
LF-5232-fix-farm-notes-modal-not-closing-when-clicking-my-profile-icon-or-feedback-button
Apr 13, 2026
Merged

LF-5232 Fix farm notes modal not closing when clicking my profile icon or feedback button#4118
SayakaOno merged 6 commits intointegrationfrom
LF-5232-fix-farm-notes-modal-not-closing-when-clicking-my-profile-icon-or-feedback-button

Conversation

@kathyavini
Copy link
Copy Markdown
Collaborator

@kathyavini kathyavini commented Apr 13, 2026

Description

This PR centralizes the visibility state for all the right side-drawers and the TopMenu Profile Menu into a single global value activeDrawer, so that a most a single drawer (or menu) can be opened at a time.

Each individual's drawer (or menu)'s open state is then derived from the global activeDrawer:

const isOpen = activeDrawer === drawerName;

The existing AppUIContext (which previously tracked isFeedbackFormOpen, setIsFeedBackFormOpen), was already perfect for holding this state, and a common exported hook (useDrawerState) can now replace localized [isOpen, setIsOpen] style state in each component.

Extending the pattern to future drawers

Now to create a new drawer that participates in this mutually exclusive application state, register the name in the appContext.ts type:

type DrawerName = 'feedbackSurvey' | 'farmNotes' | /*... */ | 'newDrawer'

and interact with it like this in the component, destructuring only the needed values

const { isOpen, openDrawer, closeDrawer, toggleDrawer } = useDrawerState('newDrawer');

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

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 ordered translation keys alphabetically (optional: run pnpm i18n to help with this)
  • I have added the GNU General Public License to all new files

This was originally created to handle a click-away listener bug on TopMenu when setActiveDrawer(null) was used with it. However, I wanted to extend to all components to decrease some of the boilerplate with the repeated activeName === drawerName etc.
@kathyavini kathyavini self-assigned this Apr 13, 2026
@kathyavini kathyavini requested review from a team as code owners April 13, 2026 20:14
@kathyavini kathyavini added bug Something isn't working enhancement New feature or request labels Apr 13, 2026
@kathyavini kathyavini requested review from SayakaOno and removed request for a team April 13, 2026 20:14
* because it verifies it is still the active drawer before applying `null`.
*/
const closeDrawer = () => {
setActiveDrawer((prev) => (prev === drawerName ? null : prev));
Copy link
Copy Markdown
Collaborator Author

@kathyavini kathyavini Apr 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The useDrawerState hook was actually created specifically to make it easier for components to use this functional setter in closeDrawer(). The original implementation which just had

setActiveDrawer(null)

on each close, produced a bug specific to the TopMenu Profile Menu, where clicking on a new drawer initiator (e.g. FeedbackForm) would trigger setActiveDrawer('feedbackForm') AND THEN also trigger the click-away listener on TopMenu, triggering its onClose() and thus setActiveDrawer(null). The result was that the feedback form would not open until you clicked again.

Right now only the TopMenu is set up with a click-away listener such that this style of onClose() is necessary, but I figured putting it here would make it safer to consume this pattern in the future, in case other participants did have click away listeners set up eventually.

And then as a bonus this hook API rather cleaned up the component logic as well!

@kathyavini kathyavini force-pushed the LF-5232-fix-farm-notes-modal-not-closing-when-clicking-my-profile-icon-or-feedback-button branch from 0dd1292 to 12fac9f Compare April 13, 2026 20:50
Copy link
Copy Markdown
Collaborator

@SayakaOno SayakaOno left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice refactor! 🙌

@SayakaOno SayakaOno added this pull request to the merge queue Apr 13, 2026
Merged via the queue into integration with commit 5029e5f Apr 13, 2026
8 checks passed
@SayakaOno SayakaOno deleted the LF-5232-fix-farm-notes-modal-not-closing-when-clicking-my-profile-icon-or-feedback-button branch April 13, 2026 21:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants