Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DSD-1337: Update Tabs mobile view #1539

Merged
merged 3 commits into from
Mar 1, 2024

Conversation

jackiequach
Copy link
Collaborator

Fixes JIRA ticket DSD-1337

This PR does the following:

  • Replaces the programmatic horizontal scrolling of mobile view with manual horizontal scrolling used on desktop view.
  • Updates functionality of arrow buttons to step through tabs. (will loop back to the start at the end and vice versa)

How has this been tested?

Accessibility concerns or updates

Checklist:

  • I have updated the Storybook documentation accordingly.
  • I have added relevant accessibility documentation for this pull request.
  • All new and existing tests passed.

Front End Review:

  • Review the Vercel preview deployment once it is ready.

Copy link

vercel bot commented Feb 28, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
nypl-design-system ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 1, 2024 4:10pm

Copy link
Member

@EdwinGuzman EdwinGuzman left a comment

Choose a reason for hiding this comment

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

This is fantastic and works so much better than my previous implementation. Thanks for this update!

Approving but check with Clare for accessibility, just want to make sure that the "scroll" text in the prev/next buttons is appropriate since this is more of focusing/moving to the next tab.

@@ -13,8 +13,6 @@ import React, { forwardRef, useState } from "react";

import Button from "../Button/Button";
import Icon from "../Icons/Icon";
import useCarouselStyles from "../../hooks/useCarouselStyles";
Copy link
Member

Choose a reason for hiding this comment

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

Can you update src/hooks/useCarouselStyles.mdx and remove the references to the Tabs component and the code snippet?

This was the only component using this so I think we can do without this hook, but that can be an update for the future.

Copy link
Collaborator

@bigfishdesign13 bigfishdesign13 left a comment

Choose a reason for hiding this comment

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

To echo what Edwin said, this looks and works great!

@bigfishdesign13 bigfishdesign13 added the Ship It Pull requests that have been reviewed and approved. label Feb 28, 2024
@jackiequach jackiequach merged commit 925be68 into react18-and-chakra Mar 1, 2024
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ship It Pull requests that have been reviewed and approved.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants