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: Allow RoutePropertiesCard to be open to "variants" or "stops" by default #2611

Merged
merged 1 commit into from
May 21, 2024

Conversation

joshlarson
Copy link
Contributor

@joshlarson joshlarson commented May 20, 2024

@joshlarson joshlarson requested a review from a team as a code owner May 20, 2024 16:57
@joshlarson joshlarson force-pushed the jdl/tests/simplify-rpc-tests branch from 6b171e7 to d12f582 Compare May 20, 2024 16:59
@joshlarson joshlarson force-pushed the jdl/feat/rpc-can-be-open-by-default branch from 0e2db63 to 11d6fc5 Compare May 20, 2024 16:59
Copy link

Coverage of commit 11d6fc5

Summary coverage rate:
  lines......: 93.6% (3242 of 3465 lines)
  functions..: 73.4% (1341 of 1828 functions)
  branches...: no data found

Files changed coverage rate: n/a

Download coverage report

@joshlarson joshlarson force-pushed the jdl/tests/simplify-rpc-tests branch from d12f582 to 47d3d4b Compare May 20, 2024 17:09
@joshlarson joshlarson force-pushed the jdl/feat/rpc-can-be-open-by-default branch from e222187 to 87575e5 Compare May 20, 2024 17:09
Copy link

Coverage of commit 87575e5

Summary coverage rate:
  lines......: 93.6% (3242 of 3465 lines)
  functions..: 73.4% (1341 of 1828 functions)
  branches...: no data found

Files changed coverage rate: n/a

Download coverage report

Copy link

Coverage of commit 87575e5

Summary coverage rate:
  lines......: 93.6% (3242 of 3465 lines)
  functions..: 73.3% (1340 of 1828 functions)
  branches...: no data found

Files changed coverage rate: n/a

Download coverage report

Comment on lines 99 to 108
export const WithVariantsOpenedByDefault: Story = {
args: {
defaultOpened: "variants",
},
}
export const WithStopsOpenedByDefault: Story = {
args: {
defaultOpened: "stops",
},
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm on the fence about including these two stories, because these stories don't really surface a UI variation of the card, but rather serve as a kind of test that the parameter of "default openness" is getting correctly applied. If we ever had a regression in that "openness" variable, the below tests would catch it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The reason I thought it was a good idea is that if anything ever changes about how the two "open" states look, Chromatic wouldn't catch it on the other stories where they default to closed.

I'm not attached to doing this - it's kind of small potatoes stakes - but that's what I was thinking when I added these.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh I seeeeeee, I see. Ok yeah, I was very easily persuaded. It makes sense to want some record of the look of those open states.

@joshlarson joshlarson force-pushed the jdl/tests/simplify-rpc-tests branch from 47d3d4b to d75281d Compare May 20, 2024 18:42
@joshlarson joshlarson changed the base branch from jdl/tests/simplify-rpc-tests to jdl/stories/story-for-route-properties-card May 20, 2024 18:42
@joshlarson joshlarson force-pushed the jdl/stories/story-for-route-properties-card branch from 5f7ae18 to f9074b3 Compare May 20, 2024 18:44
@joshlarson joshlarson force-pushed the jdl/feat/rpc-can-be-open-by-default branch from 87575e5 to fafcc08 Compare May 20, 2024 18:44
Copy link

Coverage of commit fafcc08

Summary coverage rate:
  lines......: 93.6% (3242 of 3465 lines)
  functions..: 73.4% (1341 of 1828 functions)
  branches...: no data found

Files changed coverage rate: n/a

Download coverage report

@joshlarson joshlarson force-pushed the jdl/feat/rpc-can-be-open-by-default branch from fafcc08 to 362869c Compare May 20, 2024 18:51
@joshlarson joshlarson force-pushed the jdl/stories/story-for-route-properties-card branch from 32efbe9 to 648cb8d Compare May 20, 2024 18:53
@joshlarson joshlarson force-pushed the jdl/feat/rpc-can-be-open-by-default branch 2 times, most recently from 699aed1 to 58471e1 Compare May 20, 2024 19:07
Base automatically changed from jdl/stories/story-for-route-properties-card to main May 20, 2024 19:10
@joshlarson joshlarson force-pushed the jdl/feat/rpc-can-be-open-by-default branch from 58471e1 to 7e93587 Compare May 20, 2024 19:11
Copy link

Coverage of commit 58471e1

Summary coverage rate:
  lines......: 93.6% (3242 of 3465 lines)
  functions..: 73.4% (1341 of 1828 functions)
  branches...: no data found

Files changed coverage rate: n/a

Download coverage report

@joshlarson joshlarson enabled auto-merge (squash) May 20, 2024 19:13
Copy link

Coverage of commit 7e93587

Summary coverage rate:
  lines......: 93.6% (3242 of 3465 lines)
  functions..: 73.4% (1341 of 1828 functions)
  branches...: no data found

Files changed coverage rate: n/a

Download coverage report

Copy link
Collaborator

@hannahpurcell hannahpurcell left a comment

Choose a reason for hiding this comment

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

👍

@joshlarson joshlarson merged commit b534f02 into main May 21, 2024
21 checks passed
@joshlarson joshlarson deleted the jdl/feat/rpc-can-be-open-by-default branch May 21, 2024 15:10
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.

2 participants