Skip to content

Conversation

@claudiucelfilip
Copy link
Contributor

Part of DOTDEV-258

Proposed Changes

  • move browse all links into the carousel

Why are these changes being made?

  • needed to implement the Plugins redesign

Testing Instructions

  • Go to /plugins?flags=marketplace-redesign
  • The Browse all links should render next to the carousel arrows
  • Go to /plugins
  • The Browse all links should render as before
Before (without flag) After (with flag)
Screenshot 2025-11-18 at 13 48 51 Screenshot 2025-11-18 at 13 48 59

Pre-merge Checklist

  • Has the general commit checklist been followed? (PCYsg-hS-p2)
  • Have you written new tests for your changes?
  • Have you tested the feature in Simple (P9HQHe-k8-p2), Atomic (P9HQHe-jW-p2), and self-hosted Jetpack sites (PCYsg-g6b-p2)?
  • Have you checked for TypeScript, React or other console errors?
  • Have you tested accessibility for your changes? Ensure the feature remains usable with various user agents (e.g., browsers), interfaces (e.g., keyboard navigation), and assistive technologies (e.g., screen readers) (PCYsg-S3g-p2).
  • Have you used memoizing on expensive computations? More info in Memoizing with create-selector and Using memoizing selectors and Our Approach to Data
  • Have we added the "[Status] String Freeze" label as soon as any new strings were ready for translation (p4TIVU-5Jq-p2)?
    • For UI changes, have we tested the change in various languages (for example, ES, PT, FR, or DE)? The length of text and words vary significantly between languages.
  • For changes affecting Jetpack: Have we added the "[Status] Needs Privacy Updates" label if this pull request changes what data or activity we track or use (p4TIVU-aUh-p2)?

@github-actions
Copy link

github-actions bot commented Nov 18, 2025

@claudiucelfilip claudiucelfilip self-assigned this Nov 18, 2025
@matticbot
Copy link
Contributor

matticbot commented Nov 18, 2025

This PR modifies the release build for the following Calypso Apps:

For info about this notification, see here: PCYsg-OT6-p2

  • blaze-dashboard
  • command-palette-wp-admin
  • help-center
  • notifications
  • odyssey-stats
  • wpcom-block-editor

To test WordPress.com changes, run install-plugin.sh $pluginSlug update/plugins-lp-redesign-carousel-browse-all on your sandbox.

@matticbot matticbot added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Nov 19, 2025
@claudiucelfilip claudiucelfilip requested a review from a team November 19, 2025 10:28
@claudiucelfilip claudiucelfilip force-pushed the update/plugins-lp-redesign-carousel-browse-all branch from e1e8aef to 0baa5da Compare November 19, 2025 10:29
Copilot finished reviewing on behalf of claudiucelfilip November 19, 2025 10:32
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR moves the "Browse all" link from the header section into the carousel controls as part of the plugins redesign. The link is conditionally rendered either in the header (legacy behavior) or in the carousel navigation (new design with feature flag).

  • Extracted BrowseAllAction component for reusability across different contexts
  • Updated carousel to display "Browse all" link alongside navigation buttons when useCarousel is enabled
  • Maintained backward compatibility for non-carousel views

Reviewed Changes

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

File Description
browse-all-action.tsx New extracted component that handles the "Browse all" link with analytics tracking
plugins-results-header/index.tsx Refactored to use the new BrowseAllAction component and export it for external use
plugins-results-header/style.scss Added CSS rules to control text-decoration for action links
plugins-browser-list/index.jsx Integrated BrowseAllAction with carousel and conditional rendering based on useCarousel flag

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 38 to 42
justify-content: flex-end;
align-items: center;
flex-grow: 4;
text-decoration: underline;
svg {
Copy link

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

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

The text-decoration: underline at line 41 applies to all .plugins-results-header__action elements (including both links and spans), but line 48-50 removes it unconditionally. This creates a CSS specificity conflict. Consider applying text-decoration: underline only to specific elements (e.g., links) or restructuring to avoid the override pattern. For example, move the underline style to a more specific selector like .plugins-results-header__action:not(a) or apply it conditionally based on element type.

Copilot uses AI. Check for mistakes.
? PluginsBrowserElementVariant.Extended
: PluginsBrowserElementVariant.Compact;
const shouldUseCarousel = useCarousel;
const browseAllAction = <BrowseAllAction browseAllLink={ browseAllLink } listName={ listName } />;
Copy link

Copilot AI Nov 19, 2025

Choose a reason for hiding this comment

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

The new carousel integration with controlsAction and conditional rendering of browseAllAction lacks test coverage. The existing tests in plugins-browser-list/test/index.jsx don't verify the carousel behavior with useCarousel={true} or the BrowseAllAction placement in carousel controls. Consider adding tests to verify: (1) BrowseAllAction is passed to DotPager when useCarousel is true, (2) browseAllLink is undefined in header when useCarousel is true, and (3) the component renders correctly in carousel mode.

Copilot uses AI. Check for mistakes.
Copy link
Member

@dzver dzver left a comment

Choose a reason for hiding this comment

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

LGTM. Copilot found some minor things but I think it's good to go and works fine.

Screenshot 2025-11-19 at 12 35 19 Screenshot 2025-11-19 at 12 34 58

@claudiucelfilip claudiucelfilip merged commit a637bf4 into trunk Nov 19, 2025
12 checks passed
@claudiucelfilip claudiucelfilip deleted the update/plugins-lp-redesign-carousel-browse-all branch November 19, 2025 11:47
@github-actions github-actions bot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Nov 19, 2025
@matticbot
Copy link
Contributor

Here is how your PR affects size of JS and CSS bundles shipped to the user's browser:

Sections (~211 bytes added 📈 [gzipped])

name                             parsed_size           gzip_size
plugins                               +264 B  (+0.0%)     +108 B  (+0.0%)
marketplace                           +264 B  (+0.0%)     +103 B  (+0.0%)
jetpack-cloud-plugin-management       +264 B  (+0.0%)     +108 B  (+0.0%)
a8c-for-agencies-plugins              +264 B  (+0.0%)     +108 B  (+0.0%)

Sections contain code specific for a given set of routes. Is downloaded and parsed only when a particular route is navigated to.

Legend

What is parsed and gzip size?

Parsed Size: Uncompressed size of the JS and CSS files. This much code needs to be parsed and stored in memory.
Gzip Size: Compressed size of the JS and CSS files. This much data needs to be downloaded over network.

Generated by performance advisor bot at iscalypsofastyet.com.

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.

4 participants