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

CIF-1548 - Generate CSS API file #380

Merged
merged 4 commits into from
Aug 26, 2020
Merged

CIF-1548 - Generate CSS API file #380

merged 4 commits into from
Aug 26, 2020

Conversation

herzog31
Copy link
Member

@herzog31 herzog31 commented Aug 20, 2020

Description

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • I have signed the Adobe Open Source CLA.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes and the overall coverage did not decrease.
  • All unit tests pass on CircleCi.
  • I ran all tests locally and they pass.

@herzog31 herzog31 added the enhancement New feature or request label Aug 20, 2020
@codecov
Copy link

codecov bot commented Aug 20, 2020

Codecov Report

Merging #380 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #380   +/-   ##
=========================================
  Coverage     84.81%   84.81%           
  Complexity      924      924           
=========================================
  Files           179      179           
  Lines          4570     4570           
  Branches        614      614           
=========================================
  Hits           3876     3876           
  Misses          557      557           
  Partials        137      137           
Flag Coverage Δ Complexity Δ
#integration 68.16% <ø> (ø) 686.00 <ø> (ø)
#jest 77.29% <ø> (ø) 0.00 <ø> (ø)
#karma 94.32% <ø> (ø) 0.00 <ø> (ø)
#unittests 86.07% <ø> (ø) 892.00 <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.


Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ce1abc4...32d9ca9. Read the comment docs.

Copy link
Contributor

@dplaton dplaton left a comment

Choose a reason for hiding this comment

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

We need to document this feature in the README - what are CSS API files and how to use them.

react-components/css-template.js Outdated Show resolved Hide resolved
@@ -16,7 +16,7 @@
"scripts": {
"lint": "eslint 'src/**/*.js'",
"webpack:dev": "webpack --mode=development",
"webpack:prod": "webpack --mode=production",
"webpack:prod": "webpack --mode=production && node css-template.js",
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure what you exactly want to achieve: only generate the files when we release (but they wouldn't be committed in the repo, right?) so that they are included in the NPM package? (sorry I haven't checked the recent changes to create the NPM package).

Copy link
Member Author

Choose a reason for hiding this comment

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

The templates are created in the dist folder which is published as npm package as part of the release. It's also generated for normal builds, but since the dist folder is part of .gitignore, it will never be committed.

Copy link
Contributor

@cjelger cjelger Aug 20, 2020

Choose a reason for hiding this comment

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

Aren't "normal builds" building with webpack:dev? Also if we do that for React Components, do we also want to do the same for the css styles of the HTL components?

Copy link
Member Author

@herzog31 herzog31 Aug 20, 2020

Choose a reason for hiding this comment

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

Aren't "normal builds" building with webpack:dev?

Correct :)

Also if we do that for React Components, do we also want to do the same for the css styles of the HTL components?

Those files already exist in https://github.com/adobe/aem-cif-guides-venia/tree/main/ui.frontend/src/main/styles. We could automate the generation of those files too, but it would work differently, since we would need to commit them directly to the venia project + archetype.

@dplaton dplaton merged commit dd4d636 into master Aug 26, 2020
@dplaton dplaton deleted the issue/CIF-1548 branch August 26, 2020 12:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request To Verify
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants