Skip to content

refactor: counter#1136

Merged
jorgemoya merged 2 commits intorefactor/ui-componentsfrom
refactor-counter
Jul 25, 2024
Merged

refactor: counter#1136
jorgemoya merged 2 commits intorefactor/ui-componentsfrom
refactor-counter

Conversation

@jorgemoya
Copy link
Copy Markdown
Contributor

@jorgemoya jorgemoya commented Jul 22, 2024

What/Why?

  • Remove cva

Testing

Locally

@jorgemoya jorgemoya requested a review from a team July 22, 2024 22:59
@changeset-bot
Copy link
Copy Markdown

changeset-bot Bot commented Jul 22, 2024

⚠️ No Changeset found

Latest commit: 7bc1ffc

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@vercel
Copy link
Copy Markdown

vercel Bot commented Jul 22, 2024

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

Name Status Preview Comments Updated (UTC)
catalyst-latest ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 24, 2024 7:00pm
6 Skipped Deployments
Name Status Preview Comments Updated (UTC)
catalyst ⬜️ Ignored (Inspect) Jul 24, 2024 7:00pm
catalyst-1millionproducts-store ⬜️ Ignored (Inspect) Visit Preview Jul 24, 2024 7:00pm
catalyst-au ⬜️ Ignored (Inspect) Visit Preview Jul 24, 2024 7:00pm
catalyst-test-store ⬜️ Ignored (Inspect) Visit Preview Jul 24, 2024 7:00pm
catalyst-uk ⬜️ Ignored (Inspect) Visit Preview Jul 24, 2024 7:00pm
catalyst-unstable ⬜️ Ignored (Inspect) Visit Preview Jul 24, 2024 7:00pm

Comment thread core/components/ui/counter/counter.tsx Outdated
Comment on lines +58 to +59
// eslint-disable-next-line @typescript-eslint/consistent-type-assertions, @typescript-eslint/non-nullable-type-assertion-style
useImperativeHandle(ref, () => inputRef.current as ElementRef<'input'>);
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Is there a better way to do this? I'm trying to use ref in sibling elements.

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.

Using this is fine tbh, but you can type it better like so:

type CounterRef = ElementRef<'input'> | null;

const inputRef = useRef<CounterRef>(null);

useImperativeHandle<CounterRef, CounterRef>(ref, () => inputRef.current);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This seems to work perfectly, thanks @chanceaclark

Comment thread core/components/ui/counter/counter.tsx Outdated
Comment on lines +58 to +59
// eslint-disable-next-line @typescript-eslint/consistent-type-assertions, @typescript-eslint/non-nullable-type-assertion-style
useImperativeHandle(ref, () => inputRef.current as ElementRef<'input'>);
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.

Using this is fine tbh, but you can type it better like so:

type CounterRef = ElementRef<'input'> | null;

const inputRef = useRef<CounterRef>(null);

useImperativeHandle<CounterRef, CounterRef>(ref, () => inputRef.current);

@jorgemoya jorgemoya requested a review from chanceaclark July 24, 2024 18:57
@github-actions
Copy link
Copy Markdown
Contributor

⚡️🏠 Lighthouse report

Lighthouse ran against https://catalyst-latest-p1mjxt722-bigcommerce-platform.vercel.app

🖥️ Desktop

We ran Lighthouse against the changes on a desktop and produced this report. Here's the summary:

Category Score
🟠 Performance 81
🟢 Accessibility 100
🟢 Best practices 96
🟠 SEO 82

📱 Mobile

We ran Lighthouse against the changes on a mobile and produced this report. Here's the summary:

Category Score
🟠 Performance 89
🟢 Accessibility 100
🟢 Best practices 96
🟠 SEO 85

@jorgemoya jorgemoya merged commit ab2558c into refactor/ui-components Jul 25, 2024
@jorgemoya jorgemoya deleted the refactor-counter branch July 25, 2024 15:55
jorgemoya added a commit that referenced this pull request Jul 30, 2024
bookernath pushed a commit that referenced this pull request Jul 30, 2024
github-merge-queue Bot pushed a commit that referenced this pull request Jul 30, 2024
* refactor: tag + usages (#1107)

* refactor(core): radio-group (#1106)

* fix: merge with main (#1105)

* refactor: remove file-chooser, no usages (#1110)

* refactor: carousel (#1109)

* refactor: badge (#1108)

* fix: eslint issues in tabs and badge

* refactor: gallery refactor (#1104)

* fix(core): lint issues from gallery refactor

* refactor(core): slideshow (#1111)

* refactor: message component (#1112)

* refactor: badge takes children (#1114)

* chore(core): delete skeleton (#1113)

* refactor: modal (#1115)

* refactor: button (#1116)

* fix: render cart count in badge

* refactor: datepicker uses popover primitives (#1118)

* refactor: accordions component (#1101)

* refactor(core): product card (#1117)

* refactor: input (#1131)

* refactor: textarea (#1134)

* refactor: checkbox (#1135)

* refactor: date picker (#1130)

* fix: radio group error state in pdp (#1133)

* refactor: pick list (#1139)

* fix: forward ref to button (#1140)

* refactor: select (#1143)

* fix: use correct font weight in badge (#1151)

* refactor: swatch (#1144)

* refactor: rectangle list (#1145)

* refactor: pick list props (#1146)

* refactor: radio group props (#1147)

* refactor: breadcrumbs (#1148)

* refactor: footer (#1154)

* fix: forward DatePicker ref (#1155)

* refactor: label (#1153)

* refactor: blog post card (#1152)

* refactor: rating (#1150)

* refactor: sheet (#1149)

* refactor: counter (#1136)

* refactor: header & navigation menu (#1160)

* fix: accordions props (#1162)

* fix: remove transparent overlay prop in Sheet (#1163)

* fix: gallery props (#1164)

* feat: add pageSize prop to carousels (#1165)

* chore: add changeset

* chore: update changeset

* fix: missing padding for sm viewport in quick search

* fix: close mobile nav on navigation

* fix: add aria-label to account tabs

* fix: skip blog post card regression

* fix: p wrapper for cart link

* fix: update header visual regression test

* fix: correct capitlization for tabs

* fix: change hover for on click in test

* fix: swatch test

* fix: changeset examples

* feat: add label prop to tabs

* fix: update account and register spec

* fix: update tests with latest changes

---------

Co-authored-by: Matthew Volk <matt.volk@bigcommerce.com>
Co-authored-by: Drew Pledger <drew@makeswift.com>
Co-authored-by: Daniel Almaguer <daniel.almaguer@bigcommerce.com>
Co-authored-by: Matthew Volk <volkmattj@gmail.com>
Co-authored-by: Anudeep Vattipalli <anudeep.vattipalli@bigcommerce.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.

3 participants