Skip to content

chore: updated login and farmer UIs - #123

Open
jeessh wants to merge 2 commits into
mainfrom
login-and-chores
Open

chore: updated login and farmer UIs#123
jeessh wants to merge 2 commits into
mainfrom
login-and-chores

Conversation

@jeessh

@jeessh jeessh commented Aug 24, 2026

Copy link
Copy Markdown
Member

Notion ticket link

chores

Implementation description

  • synced login so it gets autofills
  • log ins persist
  • spacing of login fixed
  • fixed edit farm page

Steps to test

  1. login flow
  2. check persistence
  3. check logging out
  4. edit farm page

What should reviewers focus on?

  • yuhyuh

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

@jeessh
jeessh requested review from rohansaha27 and a lite review from Copilot August 24, 2026 23:51
@github-actions

github-actions Bot commented Aug 24, 2026

Copy link
Copy Markdown

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

https://mississippi-farm-to-scho-11069--pr123-login-and-chores-961yphfm.web.app

(expires Tue, 01 Sep 2026 00:21:00 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 723b679f48d9c08901a0e4baad9cff58f7ce1eb0

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Updates authentication autofill and session navigation, farmer account controls, farm filtering, and form styling.

Changes:

  • Improved email autocomplete across authentication pages.
  • Added dashboard routing, account actions, and farm counts.
  • Added map filters and refined farmer form components.

Reviewed changes

Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.

Show a summary per file
File Summary Review findings
frontend/src/routes/signup/+page.svelte Updated signup email autofill. None
frontend/src/routes/reset-password/+page.svelte Updated reset-password email autofill. None
frontend/src/routes/login/+page.svelte Updated login autocomplete. None
frontend/src/routes/forgot-password/+page.svelte Updated forgot-password email autofill. None
frontend/src/routes/+page.svelte Added session-aware dashboard navigation. None
frontend/src/routes/(protected)/farmer/farms/[id]/edit/+page.svelte Refined edit-form layout and spacing. None
frontend/src/routes/(protected)/farmer/+layout.svelte Added account actions and farm counts. None
frontend/src/lib/utils/farm-tags.ts Exported tag ordering for filter options. None
frontend/src/lib/utils/farm-filters.ts Added farm filtering logic and options. None
frontend/src/lib/components/UploadZone.svelte Refreshed upload-zone styling. None
frontend/src/lib/components/TextInput.svelte Updated large input styling. None
frontend/src/lib/components/Sidebar.svelte Added account menu interactions. None
frontend/src/lib/components/map/FilterMenuPill.svelte Added reusable filter dropdown controls. None
frontend/src/lib/components/map/FarmListSidebar.svelte Applies filters to the farm list. Moderate, 3 votes: The map receives unfiltered farms while the sidebar is filtered, leaving results inconsistent and filtered farms clickable.
frontend/src/lib/components/map/FarmFilterBar.svelte Added farm filter controls. Moderate, 3 votes: The disabled Distance control prevents its explanatory hint from opening while location is pending or denied.
frontend/src/lib/components/ChoiceGroup.svelte Refreshed checkbox-group styling. None
Suppressed comments (7)

frontend/src/lib/components/Sidebar.svelte:161

  • After opening this button, focus remains on the trigger, but the menu buttons are rendered before it in the DOM (lines 120–153). Forward Tab from the trigger skips both actions, so keyboard users cannot reach Reset password or Log out normally. Move the menu after the trigger in DOM order or move focus to the first menu item on open and restore it on close.
			<button
				type="button"
				class="sidebar__profile"
				aria-haspopup="menu"
				aria-expanded={menuOpen}
				onclick={toggleMenu}

frontend/src/lib/components/TextInput.svelte:177

  • The public size prop documentation above still describes lg as having larger light labels and Nunito input text, but these new rules make the labels 15px/500 and use DM Sans. Please update that API comment with this style change so future callers are not guided by stale behavior.
	/* --- size="lg": the farm edit form, matching the add-farm page's
	   .field-label / .field so the two forms look like one product. --- */

frontend/src/lib/components/map/FarmListSidebar.svelte:44

  • This applies the new client-side filters only to the farms already loaded into farms, but /api/farms calls farms {} and the backend resolver defaults that query to a page size of 50. Once more than 50 approved farms exist, matches beyond the first page can never appear in the filtered results or count. Fetch the complete dataset or add server-side filtering/pagination before presenting these filters.
	const visibleFarms = $derived(filterFarms(farms, filters, userLocation.coords));

frontend/src/lib/components/map/FilterMenuPill.svelte:36

  • toggleOpen stops propagation, but every FilterMenuPill installs its own window click handler. Clicking another pill therefore never reaches the already-open pill's outside-click handler, so multiple panels can remain open at once. Coordinate the open pill in FarmFilterBar or remove this propagation block while preserving inside-click behavior.
	function toggleOpen(event: MouseEvent) {
		event.stopPropagation();
		if (disabled) return;
		open = !open;

frontend/src/lib/components/map/FilterMenuPill.svelte:94

  • This trigger advertises aria-haspopup="true", which means a menu, but the popup is role="group" containing checkbox/radio buttons. For multiple={false}, the options are additionally exposed as radios without a radiogroup, so assistive technologies receive inconsistent grouping/type information. Align the popup and trigger semantics (and provide the required radio-group keyboard behavior) rather than mixing menu and group patterns.
		aria-expanded={open}
		aria-haspopup="true"
		{disabled}
		onclick={toggleOpen}
	>
		{#if activeCount > 0}
			<span class="filter-menu__count">{activeCount}</span>
		{/if}
		<span class="filter-menu__label">{label}</span>
		<img
			class="filter-menu__chevron"
			class:filter-menu__chevron--open={open}
			src="/images/map/chevronDownIcon.svg"
			alt=""
		/>
	</button>

	{#if open}
		<div class="filter-menu__panel" role="group" aria-label={ariaLabel}>

frontend/src/routes/+page.svelte:28

  • The destination here is resolved from the Firebase ID-token claim/email helper, but the backend explicitly uses the Firestore users.role as the primary source (backend/utilities/authHelpers.ts:35-53). A promoted admin with a non-admin email and no refreshed claim is therefore sent to /farmer/farms by this new Dashboard link instead of /admin; use the same authoritative role source for this CTA.
				dashboardHref = (await resolveUserRole(user)) === 'ADMIN' ? '/admin' : '/farmer/farms';

frontend/src/routes/+page.svelte:28

  • signedIn is set to true before the awaited role lookup completes, while dashboardHref still has the farmer default. On a slow token lookup an admin can click Dashboard and enter the farmer route, and a rejected resolveUserRole leaves that wrong link while producing an unhandled promise rejection. Resolve the role before exposing/enabling this link and handle lookup failure with an explicit fallback.
		return onAuthStateChanged(auth, async (user) => {
			signedIn = !!user && user.emailVerified;

			if (user && signedIn) {
				dashboardHref = (await resolveUserRole(user)) === 'ADMIN' ? '/admin' : '/farmer/farms';

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +54 to +55
disabled={!locationKnown}
hint={locationKnown ? undefined : 'Allow location access to filter by distance.'}

// Everything below the filter row works off the filtered list, so the count,
// the prev/next stepper and the map selection all agree on what is visible.
const visibleFarms = $derived(filterFarms(farms, filters, userLocation.coords));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Implemented in 83a0d1c: the map now receives the same filtered farm collection shown in the sidebar by propagating visibleFarms from FarmListSidebar to the farms page and passing that list into FarmMap.

fix: keep farm map in sync with sidebar filters

Co-authored-by: jeessh <87463074+jeessh@users.noreply.github.com>

chore: update firebase hosting previews
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants