-
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
104 - Create the files table UI [1/2] #133
Conversation
… of https://github.com/IQSS/dataverse-frontend into feature/104-create-the-file-table
…ture/104-create-the-file-table
$warning: $dv-warning-color; | ||
$danger: $dv-danger-color; | ||
|
||
@import "../../../../../../node_modules/bootstrap/scss/functions"; |
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.
Have you tried to run the containerized dev env with these changes? It fails to build the design system package because it can't find the stylesheets from node_modules you import here. This same error can be found in the e2e action, which runs the dev env, as you can see here: https://github.com/IQSS/dataverse-frontend/actions/runs/5658874814/job/15331151587?pr=133
If I'm not wrong, since you introduced lerna for the package management, the node_modules from root is used for the design system package as well, and it seems that this is not compatible with the way the frontend application image is currently built: https://github.com/IQSS/dataverse-frontend/blob/develop/dev.Dockerfile#L6
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.
I haven't run the containerized dev env with these changes, my bad, thanks for checking it
I think this isn't coming from the lerna PR, because there are no changes to this file in that PR
I think this part was added ../../../../../../node_modules/
by the IDE when I moved this file to another location in this same PR, we just need to remove that part from the imports. I tried that fix and it seems to be passing the npm build step from the dockerfile.
The bad news is that now it's failing in a different step:
=> [build_image 3/9] COPY ./packages/design-system ./ 1.0s
=> [build_image 4/9] RUN npm install 162.1s
=> [build_image 5/9] RUN npm run build 22.6s
=> [build_image 6/9] WORKDIR /usr/src/app 0.0s
=> [build_image 7/9] COPY package.json ./ 0.0s
=> [build_image 8/9] COPY .npmrc ./ 0.0s
=> ERROR [build_image 9/9] RUN npm install 201.9s
------
> [build_image 9/9] RUN npm install:
#0 172.3 npm WARN deprecated [email protected]: See https://github.com/lydell/source-map-url#deprecated
#0 172.4 npm WARN deprecated [email protected]: Please see https://github.com/lydell/urix#deprecated
#0 172.5 npm WARN deprecated [email protected]: https://github.com/lydell/resolve-url#deprecated
#0 172.6 npm WARN deprecated [email protected]: Use String.prototype.trim() instead
#0 172.8 npm WARN deprecated [email protected]: See https://github.com/lydell/source-map-resolve#deprecated
#0 172.8 npm WARN deprecated @types/[email protected]: This is a stub types definition. vfile-message provides its own type definitions, so you do not need this installed.
#0 200.7 npm ERR! code 1
#0 200.7 npm ERR! path /usr/src/app/node_modules/@parcel/watcher
#0 200.7 npm ERR! command failed
#0 200.7 npm ERR! command sh -c node-gyp-build
#0 200.7 npm ERR! gyp info it worked if it ends with ok
#0 200.7 npm ERR! gyp info using [email protected]
#0 200.7 npm ERR! gyp info using [email protected] | linux | arm64
#0 200.7 npm ERR! gyp ERR! find Python
#0 200.7 npm ERR! gyp ERR! find Python Python is not set from command line or npm configuration
#0 200.7 npm ERR! gyp ERR! find Python Python is not set from environment variable PYTHON
#0 200.7 npm ERR! gyp ERR! find Python checking if "python3" can be used
#0 200.7 npm ERR! gyp ERR! find Python - "python3" is not in PATH or produced an error
#0 200.7 npm ERR! gyp ERR! find Python checking if "python" can be used
#0 200.7 npm ERR! gyp ERR! find Python - "python" is not in PATH or produced an error
#0 200.7 npm ERR! gyp ERR! find Python
#0 200.7 npm ERR! gyp ERR! find Python **********************************************************
#0 200.7 npm ERR! gyp ERR! find Python You need to install the latest version of Python.
#0 200.7 npm ERR! gyp ERR! find Python Node-gyp should be able to find and use Python. If not,
#0 200.7 npm ERR! gyp ERR! find Python you can try one of the following options:
#0 200.7 npm ERR! gyp ERR! find Python - Use the switch --python="/path/to/pythonexecutable"
#0 200.7 npm ERR! gyp ERR! find Python (accepted by both node-gyp and npm)
#0 200.7 npm ERR! gyp ERR! find Python - Set the environment variable PYTHON
#0 200.7 npm ERR! gyp ERR! find Python - Set the npm configuration variable python:
#0 200.7 npm ERR! gyp ERR! find Python npm config set python "/path/to/pythonexecutable"
#0 200.7 npm ERR! gyp ERR! find Python For more information consult the documentation at:
#0 200.7 npm ERR! gyp ERR! find Python https://github.com/nodejs/node-gyp#installation
#0 200.7 npm ERR! gyp ERR! find Python **********************************************************
#0 200.7 npm ERR! gyp ERR! find Python
#0 200.7 npm ERR! gyp ERR! configure error
#0 200.7 npm ERR! gyp ERR! stack Error: Could not find any Python installation to use
#0 200.7 npm ERR! gyp ERR! stack at PythonFinder.fail (/usr/src/app/node_modules/node-gyp/lib/find-python.js:330:47)
#0 200.7 npm ERR! gyp ERR! stack at PythonFinder.runChecks (/usr/src/app/node_modules/node-gyp/lib/find-python.js:159:21)
#0 200.7 npm ERR! gyp ERR! stack at PythonFinder.<anonymous> (/usr/src/app/node_modules/node-gyp/lib/find-python.js:202:16)
#0 200.7 npm ERR! gyp ERR! stack at PythonFinder.execFileCallback (/usr/src/app/node_modules/node-gyp/lib/find-python.js:294:16)
#0 200.7 npm ERR! gyp ERR! stack at exithandler (node:child_process:427:5)
#0 200.7 npm ERR! gyp ERR! stack at ChildProcess.errorhandler (node:child_process:439:5)
#0 200.7 npm ERR! gyp ERR! stack at ChildProcess.emit (node:events:512:28)
#0 200.7 npm ERR! gyp ERR! stack at ChildProcess._handle.onexit (node:internal/child_process:291:12)
#0 200.7 npm ERR! gyp ERR! stack at onErrorNT (node:internal/child_process:483:16)
#0 200.7 npm ERR! gyp ERR! stack at process.processTicksAndRejections (node:internal/process/task_queues:82:21)
#0 200.7 npm ERR! gyp ERR! System Linux 5.15.49-linuxkit
#0 200.7 npm ERR! gyp ERR! command "/usr/local/bin/node" "/usr/src/app/node_modules/.bin/node-gyp" "rebuild"
#0 200.7 npm ERR! gyp ERR! cwd /usr/src/app/node_modules/@parcel/watcher
#0 200.7 npm ERR! gyp ERR! node -v v19.6.1
#0 200.7 npm ERR! gyp ERR! node-gyp -v v9.4.0
#0 200.7 npm ERR! gyp ERR! not ok
#0 200.7
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.
Update: lerna has a peerDependency which needs python, Guillermo will try to add python to the container to see if that fixes the issue
At http://localhost:6006 I get a different error: And when I go to http://localhost:5173/spa it's blank. I thought there used to be a homepage there: I'm on 0abd965. I also created this issue because I recloned the repo and found the README a bit lacking: |
When merging develop the package-lock.json was updated with a version of graphql which is raising the error Check this issue mswjs/msw#1655 I brought the package-lock.json from an old commit and storybook seems to be working now. We shouldn't rebuild the package-lock.json by deleting it and running npm i, package-lock.json keeps the peerDependencies fixed |
Can you check if the issue persists after my last commits?
When I was configuring the publishing of the design-system I added the .env to the .gitignore. Can you check if you have a .env file in your root with the VITE_DATAVERSE_BACKEND_URL variable?
You're right, we need to add an explanation about creating the .npmrc file, thanks for opening the issue |
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.
Storybook now works for me. The UI content looks good.
Container image building also work (both locally and in the e2e tests action) with the latest fixes.
Action execution:
https://github.com/IQSS/dataverse-frontend/actions/runs/5713379804/job/15478670694?pr=133
The code also looks good to me, approving.
I'm on c58dfd1 and things are much better. I'm going to remove myself from this issue but I'd feel remiss if I didn't point out that the thumbnails are sometimes weird for me: Just something to look out for, perhaps. |
104 - Create the files table UI [1/2]
What this PR does / why we need it:
This PR adds the files table to the Dataset page. Part 1/2 focuses on displaying the files data so it doesn't adds filters, search bar, sorting or actions on the cells.
This is only the UI so it's not connected to the API yet. The integration will be develops in this other issue:
Which issue(s) this PR closes:
Special notes for your reviewer:
This PR also adds some changes to the 'dataverse-design-system'
Suggestions on how to test this:
This needs to be tested in Storybook since the API call is mocked:
npm i
cd packages/design-system && npm run build
npm run storybook
go to localhost:6007 and look for the Dataset page and the DatasetFiles folders
Does this PR introduce a user interface change? If mockups are available, please link/include them here:
Is there a release notes update needed for this change?:
Files table added to the Dataset page in view mode
Additional documentation: