feat: add spec pattern modal #19801

ZachJW34 · 2022-01-20T23:02:51Z

Closes UNIFY-680

User facing changelog

Clicking the "Matches" indicator on the spec list brings up spec pattern modal

Additional details

Also updates the styles of the indicator now that it is actionable.

How has the user experience changed?

PR Tasks

Have tests been added/updated?
[na] Has the original issue (or this PR, if no issue exists) been tagged with a release in ZenHub? (user-facing changes only)
[na] Has a PR for user-facing changes been opened in cypress-documentation?
[na] Have API changes been updated in the type definitions?
[na] Have new configuration options been added to the cypress.schema.json?

cypress-bot · 2022-01-20T23:02:53Z

Thanks for taking the time to open a PR!

Create a Draft Pull Request if your PR is not ready for review. Mark the PR as Ready for Review when you're ready for a Cypress team member to review the PR.
Become familiar with the Code Review Checklist for guidelines on coding standards and what needs to be done before a PR can be merged.

cypress · 2022-01-20T23:07:01Z

Test summary

17937 • 0 • 216 • 0 • 2

Run details


Project	cypress
Status	Passed
Commit	`9f47eee`
Started	Jan 28, 2022 4:31 PM
Ended	Jan 28, 2022 4:43 PM
Duration	12:11 💡
OS	Linux Debian - 10.10
Browser	Multiple

View run in Cypress Dashboard ➡️

Flakiness

	cypress/e2e/commands/net_stubbing.cy.ts		2
1	network stubbing > waiting and aliasing > can timeout waiting on a single request using "alias.request"
2	network stubbing > waiting and aliasing > can timeout waiting on a single request using "alias.request"

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

lmiller1990

one question but looks great!

lmiller1990 · 2022-01-24T02:23:21Z

packages/app/src/specs/SpecsList.vue

+      @spec-pattern="bindingsOpen = true"
+    />
+    <SpecPatternModal
+      v-if="props.gql.currentProject && bindingsOpen"


Should we combine the v-if and :show? It looks like they do same thing (conditionally hide/show?)

I think they have different purposes here. It might be better to make the v-if depend just on props.gql.currentProject so that the SpecPatternModal component renders only once, when there is a currentProject, and keep bindingsOpen to only manage the open/closed state. With the current v-if we add/remove the component from the DOM, at the same time as we are opening and closing the modal.

@marktnoonan I'll implement what you've stated above, and also rename some variables as bindingsOpen was a copy and paste and it should be named more contextually.

…tern-modal

lmiller1990 · 2022-01-25T00:54:18Z

LGTM, I will leave merging to @ZachJW34 - might be good to re-run CI, just to see if it's flake or real fails.

ZachJW34 · 2022-01-25T01:03:45Z

@lmiller1990 thanks for the review. There is a PR that was going to clash with this one that was a higher priority so I'm letting this sit until it's merged and I'll fix it up

…tern-modal

marktnoonan

Works great. I see some alignment issues in a couple of components when I look at them on my machine, but not sure they come from this PR.

One comment on an accessibility update you could make with the button, but I'm in a similar part of the code in my next PR so happy to make it then and get this merged if you like.

marktnoonan · 2022-01-27T15:41:51Z

packages/app/src/specs/SpecsListHeader.vue

+          <span
+            class="block border-l h-24px px-16px border-l-gray-100 group-hover:border-none"
+          >
+            {{ resultCount }} {{ resultCount === 1 ? t('specPage.matchSingular') : t('specPage.matchPlural') }}


Suggested change

{{ resultCount }} {{ resultCount === 1 ? t('specPage.matchSingular') : t('specPage.matchPlural') }}

{{ resultCount }} {{ resultCount === 1 ? t('specPage.matchSingular') : t('specPage.matchPlural') }}

<span class="sr-only">{{ t(`createSpec.viewSpecPatternButton`) }} </span>

If we add an sr-only span here, it will give the button a useful label for screen readers, and we can replace .get('[data-cy="open-spec-pattern-modal"]') with something like cy.contains('button', 'View spec pattern') in tests.

Updated! I noticed alignment issues as well but they disappeared and I can't reproduce them. Could have been a new class that wasn't picked up if yarn dev was still running?

that sounds right!

…tern-modal

elevatebart

in azure there was a status Approved with comments... that is what I want here.
Not perfect but should go in.

elevatebart · 2022-01-28T16:21:19Z

packages/app/src/specs/SpecsList.vue

+    <SpecPatternModal
+      v-if="props.gql.currentProject"
+      :show="showSpecPatternModal"
+      :gql="props.gql.currentProject"
+      @close="showSpecPatternModal = false"
    />


This looks like this should be added to the top component.
Then the trigger would be through a global store variable.

less repetition for something that global

* 10.0-release: (25 commits) fix(unify): improve dev server config ergonomics (#19957) feat: add spec pattern modal (#19801) fix: Windows e2e project scaffolding issues (#19938) feat: update @cypress/schematic to use proper e2e config for 10.0.0 (#19827) fix: correctly migrate projects with custom integration folder (#19929) fix: component spec creation with spec pattern (#19862) fix: missed committing yarn.lock after merge conflict fix: correct reference branch / commitSha in performance-reporter (#19941) feat: update navbar UI per Figma (#19926) fix: seed examples files when no e2e directory is created (#19768) chore: remove windy lightBlue warning test: component test updates (#19925) feat: Focus browser from select browser screen and on dashboard login (#19842) test: Honeycomb system-test reporter (#19855) fix(deps): update dependency engine.io to v5.2.1 [security] feat: Retain fileName when working with aliased fixtures and files (#19820) Update release-process.md Update release-process.md Update release-process.md Update release-process.md ...

ZachJW34 added 2 commits January 20, 2022 16:47

feat: add spec pattern modal

2299cd0

remove double question mark

4bac8df

lmiller1990 reviewed Jan 24, 2022

View reviewed changes

marktnoonan mentioned this pull request Jan 24, 2022

test(unify): Specs Page Workflows - E2E/Component #19780

Merged

2 tasks

ZachJW34 added 2 commits January 24, 2022 09:21

Merge remote-tracking branch 'origin/10.0-release' into view-spec-pat…

8bbae88

…tern-modal

refactor modal v-if and show

43030d7

lmiller1990 self-requested a review January 25, 2022 00:06

lmiller1990 previously approved these changes Jan 25, 2022

View reviewed changes

ZachJW34 added 2 commits January 25, 2022 10:21

Merge remote-tracking branch 'origin/10.0-release' into view-spec-pat…

f7c1902

…tern-modal

rebase, add modal to no specs page

96b6473

ZachJW34 dismissed lmiller1990’s stale review via 96b6473 January 25, 2022 17:45

ZachJW34 force-pushed the view-spec-pattern-modal branch from 2f1f7cc to 96b6473 Compare January 25, 2022 17:45

ZachJW34 added 7 commits January 25, 2022 13:41

Merge remote-tracking branch 'origin/10.0-release' into view-spec-pat…

86ee997

…tern-modal

fix test

40c8253

Merge remote-tracking branch 'origin/10.0-release' into view-spec-pat…

5ed7e58

…tern-modal

remove unused scroll-to-settings feature

e051c15

fix tests

718cde7

Merge remote-tracking branch 'origin/10.0-release' into view-spec-pat…

5e2cf9b

…tern-modal

e2e project dirs

309a6a8

marktnoonan previously approved these changes Jan 27, 2022

View reviewed changes

ZachJW34 added 2 commits January 28, 2022 09:32

Merge remote-tracking branch 'origin/10.0-release' into view-spec-pat…

30b97b2

…tern-modal

address comments

67bf00f

ZachJW34 dismissed marktnoonan’s stale review via 67bf00f January 28, 2022 15:59

elevatebart approved these changes Jan 28, 2022

View reviewed changes

Merge branch '10.0-release' into view-spec-pattern-modal

9f47eee

ZachJW34 merged commit 03e428f into 10.0-release Jan 28, 2022

ZachJW34 deleted the view-spec-pattern-modal branch January 28, 2022 16:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add spec pattern modal #19801

feat: add spec pattern modal #19801

ZachJW34 commented Jan 20, 2022

cypress-bot bot commented Jan 20, 2022

cypress bot commented Jan 20, 2022 •

edited

Loading

lmiller1990 left a comment

lmiller1990 Jan 24, 2022

marktnoonan Jan 24, 2022

ZachJW34 Jan 24, 2022

lmiller1990 commented Jan 25, 2022

ZachJW34 commented Jan 25, 2022

marktnoonan left a comment

marktnoonan Jan 27, 2022

ZachJW34 Jan 28, 2022

marktnoonan Jan 28, 2022

elevatebart left a comment

elevatebart Jan 28, 2022

elevatebart Jan 28, 2022

	{{ resultCount }} {{ resultCount === 1 ? t('specPage.matchSingular') : t('specPage.matchPlural') }}
	{{ resultCount }} {{ resultCount === 1 ? t('specPage.matchSingular') : t('specPage.matchPlural') }}
	<span class="sr-only">{{ t(`createSpec.viewSpecPatternButton`) }} </span>

feat: add spec pattern modal #19801

feat: add spec pattern modal #19801

Conversation

ZachJW34 commented Jan 20, 2022

User facing changelog

Additional details

How has the user experience changed?

PR Tasks

cypress-bot bot commented Jan 20, 2022

cypress bot commented Jan 20, 2022 • edited Loading

Test summary

Run details

Flakiness

lmiller1990 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lmiller1990 commented Jan 25, 2022

ZachJW34 commented Jan 25, 2022

marktnoonan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

elevatebart left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cypress bot commented Jan 20, 2022 •

edited

Loading