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

Issue#47: Install and configure sass npm package #50

Merged
merged 7 commits into from
Sep 30, 2024
Merged

Conversation

zahrafalak
Copy link
Collaborator

@zahrafalak zahrafalak commented Sep 28, 2024

Description

We need to add SCSS support to the project to improve the maintainability and organization of styles across components. SCSS offers enhanced styling capabilities, such as variables, nesting, and mixins, which will help make the code more modular and scalable.

Related Issue

Closes #47

Acceptance Criteria

  • Install the sass npm package globally via npm install -g sass.
  • Ensure SCSS files are imported and used in TypeScript components.
  • Update at least one component to demonstrate the use of SCSS.

Type of Changes

enhancement feature

Notes

  • The AuthenticatedNavBar.scss file is updated to demonstrate the use of scss. The nested CSS selectors show a clear visual hierarchy.
  • A new partial _variables.scss is added under styles folder with some placeholder values for colours.

Related document resource

@zahrafalak zahrafalak changed the base branch from fz-integrate-scss to main September 28, 2024 04:05
Copy link

github-actions bot commented Sep 28, 2024

Visit the preview URL for this PR (updated for commit ce31176):

https://tcl-77-smart-shopping-list--pr50-fz-clean-branch-lrlnktis.web.app

(expires Mon, 07 Oct 2024 04:33:38 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: b77df24030dca7d8b6561cb24957d2273c5e9d72

@zahrafalak zahrafalak self-assigned this Sep 28, 2024
Copy link
Collaborator

@alex-andria alex-andria left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I saw you wrote an example inside the acceptance criteria. But approving for now in the meantime!

@zahrafalak zahrafalak marked this pull request as ready for review September 28, 2024 20:58
@zahrafalak zahrafalak changed the title Install sass npm package and rename all css files to scss Issue#47: Install sass npm package and rename all css files to scss Sep 28, 2024
@zahrafalak zahrafalak changed the title Issue#47: Install sass npm package and rename all css files to scss Issue#47: Install and configure sass npm package Sep 28, 2024
@@ -34,6 +34,7 @@
"jsdom": "^24.1.1",
"lint-staged": "^15.2.9",
"prettier": "^3.3.3",
"sass-embedded": "^1.79.3",
Copy link
Collaborator

@bbland1 bbland1 Sep 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: when running npm start i am getting an error about sass not installed after running npm i when switching to the branch. i'm not seeing it in the devDependenceies and i just want to check if it is something i am doing wrong 😅

Screenshot 2024-09-29 at 3 58 51 AM

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh, interesting. The error doesn't show up on my end so not sure what's up. Did you try to see if it works in the preview? If it works there and doesn't work when you checkout the branch, maybe try deleting the node modules and do npm ci?

@bbland1
Copy link
Collaborator

bbland1 commented Sep 29, 2024

Thanks for doing this! I used the preview link to just see it was all still working and looking at it and it was well done. I really appreciate the example on the AuthenticatedNavBar! I've done some reading about bootstrap and sass so I think it is going to be really cool to have both of them! Seeing the changes in the className and how that will integrate with the bootstrap components is going to be quite nice. I'm excited to get the colors in there and see the process flow.

@zahrafalak
Copy link
Collaborator Author

Thanks for doing this! I used the preview link to just see it was all still working and looking at it and it was well done. I really appreciate the example on the AuthenticatedNavBar! I've done some reading about bootstrap and sass so I think it is going to be really cool to have both of them! Seeing the changes in the className and how that will integrate with the bootstrap components is going to be quite nice. I'm excited to get the colors in there and see the process flow.

Good to know that it was still working in preview 😄 Thanks for the feedback! Yeah, I do think this will work great together with the bootstrap components.

@zahrafalak zahrafalak merged commit 91d360a into main Sep 30, 2024
2 checks passed
@zahrafalak zahrafalak deleted the fz-clean-branch branch September 30, 2024 04:43
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.

Install and Configure SCSS Compilation Using the sass npm Package
3 participants