Skip to content

Feature/ad fe t2 specials navigation - #311

Open
JaferRadi wants to merge 8 commits into
DataBytes-Organisation:mainfrom
JaferRadi:feature/ad-fe-t2-specials-navigation
Open

JaferRadi wants to merge 8 commits into
DataBytes-Organisation:mainfrom
JaferRadi:feature/ad-fe-t2-specials-navigation

Conversation

@JaferRadi

Copy link
Copy Markdown

Why

This PR completes the navigation flow required for AD-FE-T2 between the Homepage Featured Specials section, the Specials page, and Product Details.

What changed

  • Added navigation from the Homepage "View All Specials" button to the Specials page.
  • Added navigation from Homepage Featured Special cards to the corresponding Product Details page.
  • Kept the existing Add to List behaviour unchanged.
  • Verified that navigation from the Specials page to Product Details was already working correctly.

Testing

  • Verified Homepage → Specials page navigation.
  • Verified Homepage Featured Special → Product Details navigation.
  • Verified Specials page → Product Details navigation.
  • Confirmed Add to List still works independently without opening Product Details.

Notes

No API, filtering, or UI design changes were made outside the scope of AD-FE-T2.

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.

🟡 Changes recommended

The PR introduces a behavioral change to how “weekly specials” are selected (and currently doesn’t sort by savings before taking the first 4), and the scope change should be corrected and/or the selection logic fixed before approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR updates the Homepage “Weekly Specials” section to support the AD-FE-T2 navigation flow between Homepage → Specials page and Homepage Featured Special card → Product Details, while keeping “Add to List” behavior independent.

Changes:

  • Added navigation from “View All Specials” to the Specials page route.
  • Made each featured special card navigable to its Product Details route (while stopping propagation from the “Add to List” button).
  • Updated the specials data loading logic to derive specials from the /products endpoint and render product images when available.
File summaries
File Description
Frontend/components/home/WeeklySpecialsSection.tsx Adds navigation to Specials and Product Details from the Homepage specials section; updates specials fetching/mapping and card rendering.
Review details
  • Files reviewed: 1/1 changed files
  • Comments generated: 3
  • Review effort level: Lite

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

Comment on lines +1 to +2
import React, { useState, useEffect } from "react";
import { View,Text,Pressable,ActivityIndicator,Image,} from "react-native";
Comment on lines +52 to 56
const response = await fetch(`${API_URL}/products?limit=50`);

if (data.success && data.data) {
setSpecials(data.data);
} else {
setError(data.error || "Failed to load weekly specials");
// Fallback to empty array or show error message
console.error("Error fetching weekly specials:", data.error);
if (!response.ok) {
throw new Error(`Products request failed: ${response.status}`);
}
(p: any) => p.product_name === product.product_name
)
)
.slice(0, 4)
@callmesoffie1811

Copy link
Copy Markdown
Collaborator

Hi @JaferRadi, I've re-reviewed the latest version. The navigation changes look good and the PR is mergeable.

There is just one thing I’d like to fix before merge: the new /products logic currently takes .slice(0, 4) before sorting by savings, so the homepage may not display the four best specials.

Could you please sort the filtered specials by savings/discount first, then take the first 4? This should be a small change.

Once updated, I’m happy for this to proceed to the next approval step.

This branch has not been deployed

No deployments
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