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

snapshot previews #73

Merged
merged 2 commits into from
Dec 29, 2017
Merged

snapshot previews #73

merged 2 commits into from
Dec 29, 2017

Conversation

bookman25
Copy link
Collaborator

Depends on snapshot support in jest-editor-support. I'd like to do some more stuff with it, but I figure this is a good first start.

package.json Outdated
"vscode:prepublish": "yarn clean-out && tsc -p ./src",
"compile": "tsc -watch -p ./src",
"vscode:prepublish": "yarn clean-out && tsc -p ./tsconfig.publish.json",
"compile": "tsc -watch -p ./tsconfig.publish.json",
Copy link
Member

Choose a reason for hiding this comment

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

interesting, what were these changes for?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Mainly because when the tsconfig was in a nested directory, I couldn't augment declarations. So the extension of jest-editor-support.d.ts wasn't working properly with a nested tsconfig.

@orta
Copy link
Member

orta commented Feb 16, 2017

We're waiting on jestjs/jest#2629 incase people are wondering

@bbigras
Copy link

bbigras commented Jun 7, 2017

jestjs/jest#2629 was closed :(

vvo pushed a commit to algolia/jest that referenced this pull request Sep 30, 2017
Following jestjs#2629, I rebased the code on current master. I still have to
actually test the feature with jest-community/vscode-jest#73.

What i would like to add ultimately is the ability to:
- preview the snapshot
- have the snapshot diff
- have a button to accept diff per snapshot (would update it)

WDYT?
cpojer pushed a commit to jestjs/jest that referenced this pull request Sep 30, 2017
* first iteration

* Use babel-traverse

* comments on the pr

* feat(jest-editor-support): Add Snapshot metadata

Following #2629, I rebased the code on current master. I still have to
actually test the feature with jest-community/vscode-jest#73.

What i would like to add ultimately is the ability to:
- preview the snapshot
- have the snapshot diff
- have a button to accept diff per snapshot (would update it)

WDYT?
@Haroenv
Copy link
Contributor

Haroenv commented Oct 2, 2017

jestjs/jest#4570 has been merged 💌

tabrindle pushed a commit to tabrindle/jest that referenced this pull request Oct 2, 2017
* first iteration

* Use babel-traverse

* comments on the pr

* feat(jest-editor-support): Add Snapshot metadata

Following jestjs#2629, I rebased the code on current master. I still have to
actually test the feature with jest-community/vscode-jest#73.

What i would like to add ultimately is the ability to:
- preview the snapshot
- have the snapshot diff
- have a button to accept diff per snapshot (would update it)

WDYT?
@orta
Copy link
Member

orta commented Oct 5, 2017

An alternative, as of the vscode out today, could be that we use a styled hover that shows the string as a colored diff

screen shot 2017-10-05 at 3 38 14 pm

@bookman25
Copy link
Collaborator Author

Maybe I'm mistaken but it looks like completion items are for when you type things. So you'd have to type out text in order to trigger the snapshot previews?

@orta
Copy link
Member

orta commented Oct 6, 2017

Ah, yeah, more specifically I was thinking about being able to maybe override the hover popup when you highlight the "toMatchSnapshot" with a preview

@bookman25
Copy link
Collaborator Author

@orta anything still pending here?

@orta
Copy link
Member

orta commented Oct 28, 2017

If this is working (with the rebased PR), then nope 👍 should be good to go IMO

@kizu
Copy link

kizu commented Dec 22, 2017

I hope this would be finished and merged one day, as it is a thing I'd really want to see!

@bookman25
Copy link
Collaborator Author

bookman25 commented Dec 29, 2017

@orta I think this should be good now. Rebased and appears to still be working locally :) So if it looks good, you want to merge?

@orta
Copy link
Member

orta commented Dec 29, 2017

Alright, will give it a look through now

@orta
Copy link
Member

orta commented Dec 29, 2017

@bookman25 - this looks 👍

screen shot 2017-12-29 at 12 19 00 pm

I'll happily merge and add this, can you add a way to disable it (as a setting) then this is good to go - congrats on bringing this PR back from the grave

"*.json": ["npm run prettier-write --", "git add"],
"*.ts": ["npm run prettier-write --", "git add"]
"*.json": ["yarn prettier-write --", "git add"],
"*.ts": ["yarn prettier-write --", "git add"]
Copy link
Member

Choose a reason for hiding this comment

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

👍

@orta orta merged commit 320fbe8 into jest-community:master Dec 29, 2017
legend1202 pushed a commit to legend1202/vscode-jest that referenced this pull request Jun 18, 2023
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.

5 participants