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

feat(components): Add testID to Divider and dynamically reference size and direction props #2137

Merged
merged 3 commits into from
Nov 26, 2024

Conversation

taylorvnoj
Copy link
Contributor

@taylorvnoj taylorvnoj commented Nov 22, 2024

Motivations

Updating Divider in @jobber/components as part of our continued effort to increase prop consistency between equivalent web and mobile components.

Now Divider props are exactly the same in mobile and web versions

Changes

Added

A new testID prop exists on the div. A test for that prop was also added in Divider.test.tsx

Changed

In following a similar pattern to other components in Atlantis, I decided to dynamically reference the size and direction props to ensure the component is tightly coupled to the available size/direction options. This means a few new files were created - DividerSizes.module.css and DividerDirections.module.css. This also DRYs up a bit of the logic in Divider.tsx

Testing

Testing the pre-release in product is linked below.
UI/UX wise, everything remains the same as it was before. You can now add a testID .


In Atlantis we use Github's built in pull request reviews.

Random photo of Atlantis

@taylorvnoj taylorvnoj self-assigned this Nov 22, 2024
Copy link

Published Pre-release for cb11119 with versions:

  - @jobber/[email protected]+cb111193

To install the new version(s) for Web run:

npm install @jobber/[email protected]+cb111193

@@ -3,7 +3,7 @@
exports[`Divider should render 1`] = `
<div>
<div
class="divider horizontal"
class="divider base horizontal"
Copy link
Contributor Author

@taylorvnoj taylorvnoj Nov 22, 2024

Choose a reason for hiding this comment

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

The word "base" was added to the snapshot test because the css was refactored to include a base size (and base is default)

@taylorvnoj taylorvnoj marked this pull request as ready for review November 22, 2024 15:19
@taylorvnoj taylorvnoj requested a review from a team as a code owner November 22, 2024 15:19
Copy link
Contributor

@Aiden-Brine Aiden-Brine 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 a really nice improvement 👍

@taylorvnoj taylorvnoj merged commit e919357 into master Nov 26, 2024
22 checks passed
@taylorvnoj taylorvnoj deleted the JOB-97733/JOB-110081/Divider-prop-cleanup branch November 26, 2024 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants