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.

Sign up for GitHub

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

Jump to bottom

[ADF-5567] Removed important property from 'permission-list.component.scss' in ADF #9144

Closed
wants to merge 12 commits into from
Closed

[ADF-5567] Removed important property from 'permission-list.component.scss' in ADF #9144

wants to merge 12 commits into from

Conversation

Yasa-Nataliya
Copy link
Contributor

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)
use !important for styles should be disallowed.

What is the new behaviour?
I have removed 'important' property from ''permission-list.component.scss''. After removing the important property I tested ACA, ADF, and ADW, and it’s working as expected.

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:
https://alfresco.atlassian.net/browse/ADF-5567

AleksanderSklorz
AleksanderSklorz reviewed Dec 13, 2023
View reviewed changes
...ervices/src/lib/permission-manager/components/permission-list/permission-list.component.scss Outdated
@@ -92,3 +85,12 @@
box-shadow: 0 8px 9px -5px var(--adf-theme-foreground-divider-color), 0 15px 22px 2px var(--adf-theme-foreground-divider-color);
}
}

.adf-datatable .adf-datatable-row .adf-datatable-cell-value .adf-permission-role-column-header {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are you overriding these styles here instead of in component where adf-permission-role-column-header class is used? This class is not in html of permission-list component. You can override styles for adf-permission-role-column-header in component which has that class in html.

AleksanderSklorz
AleksanderSklorz reviewed Dec 13, 2023
View reviewed changes
...ervices/src/lib/permission-manager/components/permission-list/permission-list.component.scss Outdated

.adf-datatable .adf-datatable-row .adf-datatable-cell-value .adf-permission-role-column-header {
position: relative;
height: 40px;
Copy link
Contributor

Choose a reason for hiding this comment

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

Height style seems not working because of some reasons. If you check console it is grayed out:
image

AleksanderSklorz
AleksanderSklorz reviewed Dec 13, 2023
View reviewed changes
...ervices/src/lib/permission-manager/components/permission-list/permission-list.component.scss Outdated
position: relative;
height: 40px;

.mat-form-field-infix {
Copy link
Contributor

Choose a reason for hiding this comment

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

We should not override styles for material classes.

MichalFidor and others added 10 commits December 14, 2023 13:43
@MichalFidor
[AAE-19026] Fix app permissions (#9169)
1f5af31
@alep85
[AAE-18459] Fix invalidateSession is called when auth is not initiliz…
0932a3e 
…ed by js-api (#9152)

* [AAE-18459] Replace all placeholders if the configuration key is an object

* [AAE-18459] don't invalidate session if authentication is not handled by js-api
@MichalFidor
[AAE-19026] Fix app permissions (#9171)
7c924e0
@Yasa-Nataliya
[ACS-5567]Implemented the changes as per review comments
ff03964
@MichalFidor @Yasa-Nataliya
[AAE-19026] Fix app permissions (#9169)
ae18a02
@alep85 @Yasa-Nataliya
[AAE-18459] Fix invalidateSession is called when auth is not initiliz…
e0e2d82 
…ed by js-api (#9152)

* [AAE-18459] Replace all placeholders if the configuration key is an object

* [AAE-18459] don't invalidate session if authentication is not handled by js-api
@Yasa-Nataliya
[ADF-5567] removed the important property
f3245e1
@Yasa-Nataliya
[ADF-5567] modified the changes
e83e9e4
@Yasa-Nataliya
[ACS-5567]Implemented the changes as per review comments
bac848e
@Yasa-Nataliya
Merge branch 'ADF-5567' of https://github.com/Alfresco/alfresco-ng2-c…
15feec7 
…omponents into ADF-5567
@sonarqubecloud SonarQubeCloud
Copy link

Quality Gate Passed Quality Gate passed

Kudos, no new issues were introduced!

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

See analysis details on SonarCloud

@Yasa-Nataliya
Copy link
Contributor Author

As this PR contains some other commits as well, closing it raises another PR: #9184

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AleksanderSklorz AleksanderSklorz AleksanderSklorz left review comments

@rbahirsheth rbahirsheth Awaiting requested review from rbahirsheth

@arohilaGL arohilaGL Awaiting requested review from arohilaGL

@eromano eromano Awaiting requested review from eromano eromano is a code owner

@popovicsandras popovicsandras Awaiting requested review from popovicsandras popovicsandras is a code owner

@DenysVuika DenysVuika Awaiting requested review from DenysVuika DenysVuika is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Yasa-Nataliya @AleksanderSklorz @MichalFidor @alep85