-
Notifications
You must be signed in to change notification settings - Fork 16
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
12 - Add version number to footer #85
Conversation
Just a few minutes ago @poikilotherm got a Dataverse container running in GitHub Actions to file upload. Check this out: IQSS/dataverse-sample-data@master...container-test Here's where you can see collections, datasets, and files being created: https://github.com/IQSS/dataverse-sample-data/actions/runs/4872123218/jobs/8689917400#step:8:72 This means it's probably possible to get the e2e test working! |
This looks great. I can see how #65 will fix the e2e tests in a local dev environment, will that fix the e2e test that runs in the github action? Or is that considered a separate issue from this PR? |
I'm also interested in understanding why the e2e test is failing. In addition, I'm wondering why the "Hello Dataverse" story doesn't have a footer at all. Should it? I took a quick swing at adding it (locally) but I must be doing it wrong because the version isn't there and it's weirdly centered: This is what I tried:
I noticed that the footer is not weirdly centered and correctly shows the version in the "Logged In" layout story: |
@pdurbin I don't think the Hello Dataverse page was meant to have the footer, it was just a test page made for the original project setup. The Footer is meant to be part of the Layout component, so maybe there are styles in that component that affect the Footer placement. |
Actually, I see the footer now in Storybook. Maybe it was just hidden by the "Controls, Actions" thing at the bottom. I had to scroll down. No version though. Is the version supposed to be there in the footer in Storybook? The page does use I do see the version in the footer at https://dev1.dataverse.org/spa/ where I have the following PR deployed: I'm not sure why because I don't see any commits about a version in the footer in that PR. It looks like this: The more important thing is the failing e2e test though, I'd say. 😄 |
The footer is there but there is scrolling, there shouldn't be a scroll. I fixed the height so now the footer it's always at the bottom without scrolling. Unless of course, that the body content is larger than the viewport. |
I did some changes with the help of Guillermo so now the e2e tests for the version number are working both locally and in the Github action The tests for the login/logout are failing because we do not pass the user credentials. I was looking at the js-dataverse integration tests and there are no tests for this, there is a todo I suggest that we either skip the e2e test for login/logout and add a TODO, as was done in the js-dataverse repo, or let it fail. I forgot to mention, the GitHub action is working because I'm pointing to demo.dataverse to get the version number, of course we should change this to the testing environment at some point. |
…averse-frontend into feature/12-add-version-number-to-footer
I think it's fine to skip the e2e tests and add a TODO, because the container environment is almost ready. Once this is merged (#87) will we be able to add the e2e test? |
…rontend into feature/12-add-version-number-to-footer
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.
looks good, passing all tests now that e2e has been removed
@kcondon just a heads up that this PR is currently targeting the |
@MellyGray you were right! "Base automatically changed from feature/53-login-logout to develop." I had no idea GitHub did this. Interesting! |
…-footer 12 - Add version number to footer
What this PR does / why we need it:
This PR adds the version number to the footer using the js-dataverse module.
Which issue(s) this PR closes:
Special notes for your reviewer:
Version number is only going to appear if you are logged in in a local dataverse installation. See this PR for more information on how to test this.
Apart from the version number I fixed some issues with the login/logout
Suggestions on how to test this:
To test this you need a Dataverse local installation connected to the frontend. See this PR for more information on how to test this.
For testing the mocked js-module api calls:
npm run storybook
If you go to the Layout page in storybook you can see the version number because the api call has been mocked.
Same with the cypress component tests and vitest tests, api calls are mocked.
Github Action for the e2e tests is not working because we don't have a way to test this, check this opened issue #65 for more information
Does this PR introduce a user interface change? If mockups are available, please link/include them here:
No
Is there a release notes update needed for this change?:
Now we can obtain the dataverse version number from the js-dataverse-client and show it at the frontend footer.
Additional documentation: