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

Refactored annot-columns component so it's ready for the store #249

Merged
merged 4 commits into from
Dec 12, 2022

Conversation

rmanaem
Copy link
Contributor

@rmanaem rmanaem commented Dec 9, 2022

Closes #248

@rmanaem rmanaem added the maint:refactor Simplifying or restructuring existing code or documentation. label Dec 9, 2022
@rmanaem rmanaem requested a review from surchs December 9, 2022 21:11
@rmanaem rmanaem self-assigned this Dec 9, 2022
Copy link
Contributor

@surchs surchs left a comment

Choose a reason for hiding this comment

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

Thanks @rmanaem! Really nice, clear, and to the point 🎉 . I think this is good to go, but I added some suggestions you can look at. The only really relevant one is whether you can reuse one of the mutations we had already thought of (and have listed on the miro board). Since we're already at it, might save a little bit of refactoring later ...

cypress/component/annot-columns.cy.js Show resolved Hide resolved
cypress/component/annot-columns.cy.js Show resolved Hide resolved
cypress/component/annot-columns.cy.js Outdated Show resolved Hide resolved
cypress/component/annot-columns.cy.js Outdated Show resolved Hide resolved
- Modified the component alias to annotColumns
- Added activeCategory as input to the removeColumn mutation
Copy link
Contributor

@surchs surchs left a comment

Choose a reason for hiding this comment

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

Awesome. Looks good to me @rmanaem! 🎉 ready to merge

@rmanaem rmanaem merged commit 6769460 into dev_components_talk_to_store_directly Dec 12, 2022
@rmanaem rmanaem deleted the maint-248 branch December 12, 2022 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maint:refactor Simplifying or restructuring existing code or documentation.
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

2 participants