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

Injecting versions [WIP] #10391

Closed
wants to merge 2 commits into from
Closed

Conversation

jacob-nv
Copy link
Contributor

@jacob-nv jacob-nv commented Jan 22, 2024

Description

SideBar.vue fetches versions which are provided to FileVersions.vue and FileSideBar.vue

Related Issue

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests
  • Documentation
  • Maintenance (e.g. dependency updates or tooling)

@jacob-nv jacob-nv requested a review from JammingBen January 22, 2024 16:23
@jacob-nv jacob-nv self-assigned this Jan 22, 2024
Copy link

update-docs bot commented Jan 22, 2024

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

@jacob-nv jacob-nv changed the title Injecting versions Injecting versions [WIP] Jan 22, 2024
@jacob-nv
Copy link
Contributor Author

@JammingBen Any recommendation for how to handle tests? Lots of the testing is based around fetching. Here is everything provided except one instance where restoring old version refreshes versions through provided method

@JammingBen
Copy link
Contributor

@JammingBen Any recommendation for how to handle tests? Lots of the testing is based around fetching. Here is everything provided except one instance where restoring old version refreshes versions through provided method

Hmm I didn't think about the re-fetching, not only from a unit test perspective, but the actual implementation. The fact that the versions panel needs to reload versions after reverting kinda makes me question the provide/inject approach... @kulmann @dschmidt what do you think?

@kulmann
Copy link
Contributor

kulmann commented Jan 29, 2024

@JammingBen Any recommendation for how to handle tests? Lots of the testing is based around fetching. Here is everything provided except one instance where restoring old version refreshes versions through provided method

Hmm I didn't think about the re-fetching, not only from a unit test perspective, but the actual implementation. The fact that the versions panel needs to reload versions after reverting kinda makes me question the provide/inject approach... @kulmann @dschmidt what do you think?

Sounds to me like a pinia store (similar to the shares pinia store implementation) would make sense. 🤔

@JammingBen
Copy link
Contributor

Closing here since this approach of fixing it is not really feasible because of the reasons mentioned above.

@JammingBen JammingBen closed this Mar 18, 2024
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.

Stop loading versions separately in each sidebar panel
3 participants