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

[ACS-6140] - Remove references to internal Angular Material CSS classes #3620

Conversation

dominikiwanekhyland
Copy link
Contributor

@dominikiwanekhyland dominikiwanekhyland commented Jan 30, 2024

Please check if the PR fulfills these requirements

  • The commit message follows our guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

What kind of change does this PR introduce? (check one with "x")

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation
  • Other... Please describe:

What is the current behaviour? (You can also link to an open issue here)

https://alfresco.atlassian.net/browse/ACS-6140

What is the new behaviour?
All internal Angular Material CSS classes are removed and replaced with our custom classes. Changes in Angular Material library with classes won't affect our app.
Also it contains adf-upstream

Does this PR introduce a breaking change? (check one with "x")

  • Yes
  • No

If this PR contains a breaking change, please describe the impact and migration path for existing applications: ...

Other information:

@dominikiwanekhyland dominikiwanekhyland force-pushed the ACS-6140-adf-remove-references-to-internal-angular-material-css-classes branch from 64af0e4 to 315cb33 Compare February 2, 2024 10:25
@@ -1,3 +1,4 @@
/* stylelint-disable selector-class-pattern */
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you guys think it's allowable to disable that styleline rule for theme files? I think it's really difficult to modify those files as they mainly focus on adapting mat classes

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's okay for me

@dominikiwanekhyland dominikiwanekhyland force-pushed the ACS-6140-adf-remove-references-to-internal-angular-material-css-classes branch 7 times, most recently from 3cfd407 to bbd5997 Compare February 13, 2024 10:51
@dominikiwanekhyland dominikiwanekhyland force-pushed the ACS-6140-adf-remove-references-to-internal-angular-material-css-classes branch from bbd5997 to 35a3600 Compare February 13, 2024 10:52
@dominikiwanekhyland dominikiwanekhyland force-pushed the ACS-6140-adf-remove-references-to-internal-angular-material-css-classes branch from 35a3600 to 33322b3 Compare February 13, 2024 10:54
Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

Copy link
Contributor

@MichalKinas MichalKinas left a comment

Choose a reason for hiding this comment

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

LGTM, one final question did you run ACA with the latest ADF version to check styling that you've changed?

@dominikiwanekhyland
Copy link
Contributor Author

Yes, I have, 2 times:

  1. while making changes, I was working on ACA and ADW with adf configurations, to see if my changes don't break anything.
  2. before creating this PR I have also walk through all of those places which have changed

And I didn't find anything suspicious :)

@dominikiwanekhyland dominikiwanekhyland merged commit 184ad17 into develop Feb 13, 2024
27 checks passed
@dominikiwanekhyland dominikiwanekhyland deleted the ACS-6140-adf-remove-references-to-internal-angular-material-css-classes branch February 13, 2024 12:22
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.

3 participants