Skip to content
This repository has been archived by the owner on Jan 9, 2023. It is now read-only.

Callout css does not work when importing to frontend #569

Closed
jackcmeyer opened this issue Aug 28, 2020 · 9 comments · Fixed by #581 or #593
Closed

Callout css does not work when importing to frontend #569

jackcmeyer opened this issue Aug 28, 2020 · 9 comments · Fixed by #581 or #593
Labels
bug Something isn't working help wanted Extra attention is needed released

Comments

@jackcmeyer
Copy link
Member

🐛 Bug Report

When trying to use the Callout component in the @hospitalrun/frontend, the CSS does not work.

To Reproduce

Steps to reproduce the behavior:

  1. Import Callout component to some component
  2. Try and use the info, danger, or warning color
  3. See that it is not styled properly
@jackcmeyer jackcmeyer added bug Something isn't working help wanted Extra attention is needed labels Aug 28, 2020
@DouglasDev
Copy link
Contributor

DouglasDev commented Aug 28, 2020

@jackcmeyer I would like to try solving this, but I'm not exactly sure how to test it. Since the front end and component libraries are separate repos, how can I test what effect the changes to the component library have on the front end? This seems tricky since they are running on completely separate dev servers.

At first glance, it seems that the SCSS isn't being imported or compiled correctly in the build files, either due to a configuration issue with Webpack, or possibly due to an issue with the structure of the class names. The only components that have their own SCSS files are the callout and the toaster. Comparing the two files, it seems that the toaster follows a different naming convention (BEM) than the callout. The toaster uses selectors like div.Toastify__toast.Toastify__toast--success whereas the (broken) callout selectors are like .callout-success. Not sure if this is related to the bug or not.

@jackcmeyer
Copy link
Member Author

Checkout the npm link command. You can use this to link a local build of the components repo to your hospitalrun-frontend project.

@DouglasDev
Copy link
Contributor

@jackcmeyer One other question: this is the first project I've worked on where the components and main front end codebase are separate repos. Is it possible to enable live reloading between repos? Meaning that if I change a component in the component repo, the front end updates that component immediately? Or do I need to rebuild the project and relink it every time there is a change? Thanks.

@jackcmeyer
Copy link
Member Author

Once you link it, every time you rebuild it should pick up the changes. You should be able to run the components repo in watch mode, so I think you should get a near live reload experience. I believe the only thing you'd need to do is restart frontend repo.

@DouglasDev
Copy link
Contributor

DouglasDev commented Sep 8, 2020

@jackcmeyer I finally figured out how to link the two repos. It was not straightforward and involved several steps. Perhaps we should add some instructions to the readme? In case anyone is wondering these were the steps I took:

  1. cd to the components repo and run npm link
  2. delete "@hospitalrun/components": "~1.16.0" from the dependencies for the front end repo (this should not be committed)
  3. cd to the front end repo and run npm link @hospitalrun/components
  4. run npm i in the front end repo and restart the dev server.
  5. You might now see an error message when you go to localhost:3000:
Hooks can only be called inside the body of a function component.
There are three common reasons you might be seeing it:

You might have mismatching versions of React and React DOM.
You might be breaking the Rules of Hooks.
You might have more than one copy of React in the same app.

The reason you are seeing this message is because the component library is trying to run React from it's own node_modules folder, when it should be running the same copy as the front-end repo. To fix this you should:

  1. run npm install npm-link-shared -g
  2. Add the following script to the front-end repo: "prestart": "npm-link-shared ./node_modules/@hospitalrun/components/node_modules . react"
    This will tell npm to use the front end repo's version of React for both the front end and the component library when you run npm run start
  3. Restart the dev server and the 2 repos should now be linked correctly.

Let me know if you have a simpler way of doing this. This is just what worked for me.

@fox1t
Copy link
Member

fox1t commented Sep 8, 2020

@jackcmeyer I finally figured out how to link the two repos. It was not straightforward and involved several steps. Perhaps we should add some instructions to the readme? In case anyone is wondering these were the steps I took:

1. cd to the components repo and run `npm link`

2. delete `"@hospitalrun/components": "~1.16.0"` from the dependencies for the front end repo (this should not be committed)

3. cd to the front end repo and run `npm link @hospitalrun/components`

4. run `npm i` in the front end repo and restart the dev server.

5. You might now see an error message when you go to localhost:3000:
Hooks can only be called inside the body of a function component.
There are three common reasons you might be seeing it:

You might have mismatching versions of React and React DOM.
You might be breaking the Rules of Hooks.
You might have more than one copy of React in the same app.

The reason you are seeing this message is because the component library is trying to run React from it's own node_modules folder, when it should be running the same copy as the front-end repo. To fix this you should:

1. run `npm install npm-link-shared -g`

2. Add the following script to the front-end repo: `"prestart": "npm-link-shared ./node_modules/@hospitalrun/components/node_modules . react"`
   This will tell npm to use the front end repo's version of React for both the front end and the component library when you run `npm run start`

3. Restart the dev server and the 2 repos should now be linked correctly.

Let me know if you have a simpler way of doing this. This is just what worked for me.

"@hospitalrun/components" is supposed to work in the workspaces environment. We are waiting for npm v7 to test npm workspaces. As of today, we are using yarn v1 workspaces but they are broken at the moment.
Your findings are awesome and we should take them as a starting point. Would you like to work on a PR that explores how to fix workspaces? There are 3 possible candidates: yarn v2 workspaces; npm v7 workspaces; pnpm workspaces.

@fox1t fox1t closed this as completed in #581 Sep 8, 2020
@ghost
Copy link

ghost commented Sep 8, 2020

🎉 This issue has been resolved in version 2.0.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@ghost ghost added the released label Sep 8, 2020
@DouglasDev
Copy link
Contributor

@fox1t Yes I can take a look at that over the weekend.

@jackcmeyer jackcmeyer reopened this Sep 8, 2020
CreativeCreate added a commit to CreativeCreate/components that referenced this issue Sep 14, 2020
…also relate to HospitalRun#254 HospitalRun#569)

Ship components package with header injected styles (also relate to "fix HospitalRun#254" "fix HospitalRun#569"). We
inject  SCSS as <style> into <head> including bootstrap styles during the build process. This way we
can skip shipping & referencing of .scss files. Please note: now that there's no need for a .scss
references in the front-end, the main.scss reference in the front-end must be removed.

BREAKING CHANGE: we skip shipping & referencing of main.scss files. Now that there's no need for a
.scss references in the front-end, the main.scss reference in the front-end must be removed.

"fix HospitalRun#254", "fix HospitalRun#569"
jackcmeyer pushed a commit that referenced this issue Sep 15, 2020
Ship components package with header injected styles (also relate to "fix #254" "fix #569"). We
inject  SCSS as <style> into <head> including bootstrap styles during the build process. This way we
can skip shipping & referencing of .scss files. Please note: now that there's no need for a .scss
references in the front-end, the main.scss reference in the front-end must be removed.

BREAKING CHANGE: we skip shipping & referencing of main.scss files. Now that there's no need for a
.scss references in the front-end, the main.scss reference in the front-end must be removed.

"fix #254", "fix #569"
ghost pushed a commit that referenced this issue Sep 15, 2020
# [3.0.0](v2.0.1...v3.0.0) (2020-09-15)

### Bug Fixes

* **component:** ship components package with header injected styles ([afee9ff](afee9ff)), closes [#254](#254) [#569](#569) [#254](#254) [#569](#569)

### BREAKING CHANGES

* **component:** we skip shipping & referencing of main.scss files. Now that there's no need for a
.scss references in the front-end, the main.scss reference in the front-end must be removed.
@ghost
Copy link

ghost commented Sep 15, 2020

🎉 This issue has been resolved in version 3.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working help wanted Extra attention is needed released
Projects
None yet
3 participants