-
Notifications
You must be signed in to change notification settings - Fork 10
feat: added async link generator to show/hide button #399
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
base: main
Are you sure you want to change the base?
Conversation
…ository files Signed-off-by: Alejandro Parcet <[email protected]>
SPARC Vue3 Portal Testing
|
||||||||||||||||||||||||||||
| Project |
SPARC Vue3 Portal Testing
|
| Branch Review |
add-osparc-link-generator
|
| Run status |
|
| Run duration | 16m 32s |
| Commit |
|
| Committer | Alejandro Parcet Gonzalez |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
|
|
0
|
|
|
0
|
|
|
39
|
|
|
0
|
|
|
170
|
| View all changes introduced in this branch ↗︎ | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds conditional visibility for the "Run on Osparc" button based on the presence of a .hornet/manifest.json file in the dataset repository. The button is now shown/hidden asynchronously by checking for the file's existence via the Pennsieve API.
Key changes:
- Replaced static
osparcLinkcomputed property with asyncgetOsparcLink()method that checks for manifest file existence - Modified button visibility to conditionally display based on API response
- Updated component styling to use consistent button widths
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| pages/datasets/[datasetId].vue | Added async link generation logic and replaced computed property with async method |
| components/DatasetDetails/SourceCodeInfo.vue | Updated button visibility logic and styling for consistent width handling |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
Co-authored-by: Copilot <[email protected]>
Signed-off-by: Alejandro Parcet <[email protected]>
Signed-off-by: Alejandro Parcet <[email protected]>
|
With this last change we introduce two global variables to:
In case no globals are added to the environment variables, the button will be enabled and no cad_manifest.json will be searched for on pennsive. This PR can be merged as soon as @pcrespov enables the new API endpoint on osparc side, or sooner with the Global variables setted to disable the button for now. |
Everything ready on the osparc API |
egauzens
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@alexpargon Just a couple comments to address. Also, is there a dataset id that I can test this on?
Signed-off-by: Alejandro Parcet <[email protected]>
Hello @egauzens ! I just solved the comments 😄. About testing, right now as far as i know, no dataset has the proper |
Hey @alexpargon I unresolved the conversations since I did not see the requested changes applied. Perhaps you forgot to push them? |
You were right! 🤯 |
Button visibility depends on repository files.
Do not merge until the Pensive API
https://api.pennsieve.net/discover/datasets/${this.datasetId}/versions/1/assets/browse?path=&limit=5&offset=0&file=.hornet/manifest.jsonIs in production, as it will not allow you to view/test the button