Financial-Times / x-dash

:x::heavy_minus_sign::newspaper: shared front-end components for FT.com and the FT Apps
https://financial-times.github.io/x-dash
38 stars 6 forks source link

ADSDEV-1117 fix broken x-privacy-manager stylesheet #667

Closed alevito closed 2 years ago

alevito commented 2 years ago

References

Jira ticket: ADSDEV-1117 Previous PR: https://github.com/Financial-Times/x-dash/commit/1b0c2251e79a51f4b11519ca8f74463db2ebea4b

Description

x-privacy-manager @ v8 has been left with a couple of broken CSS rules since the introduction of these changes https://github.com/Financial-Times/x-dash/commit/1b0c2251e79a51f4b11519ca8f74463db2ebea4b

Also, note that in x-privacy-manager v8 also the Accept/Block button does not change colour when the user clicks on it.

x-dash components at v8 have been updated to the latest Origami components, therefore we need to update the dependency x-privacy-manager to v8 in the consuming apps - i.e., next-control-centre and ft-app.

The purpose of this PR is to fix the x-privacy-manager v8 stylesheet in order to make it look exactly as in v7.

This is how the component looks in x-privacy-manager@7 and current production.

Screenshot 2022-06-22 at 17 53 55

This is how the component looks in x-privacy-manager@8 - not deployed to consuming apps.

Screenshot 2022-06-22 at 17 57 14

Notes

alevito commented 2 years ago

@debugwand

wouldn't this result in the rule .x-privacy-manager-radio-button. .x-privacy-manager-radio-button?

.x-privacy-manager-radio-button { flex: 1;

& + .x-privacy-manager-radio-button {

Not exactly. The plus sign is the adjacent siblings combinator, so its purpose is to target the element adjacent to a previous element, as in

<div>
    <p />
    <p /> // a selector .p + .p selects this second p element
</div>

In x-privacy-manager@7 this was done as follows. Its purpose is to add styling specific to the second radio button (the Block button).

.control {
   ...
   & + .control {
      ...
   }
}

See https://github.com/Financial-Times/x-dash/blob/dda519f5639d0423317e3e0ec91eb0497eae0884/components/x-privacy-manager/src/components/radio-btn.scss#L13

This commit here introduced the bug https://github.com/Financial-Times/x-dash/commit/1b0c2251e79a51f4b11519ca8f74463db2ebea4b

debugwand commented 2 years ago

@debugwand

wouldn't this result in the rule .x-privacy-manager-radio-button. .x-privacy-manager-radio-button? .x-privacy-manager-radio-button { flex: 1;

& + .x-privacy-manager-radio-button {

Not exactly. The plus sign is the adjacent siblings combinator, so its purpose is to target the element adjacent to a previous element, as in

<div>
    <p />
    <p /> // a selector .p + .p selects this second p element
</div>

In x-privacy-manager@7 this was done as follows. Its purpose is to add styling specific to the second radio button (the Block button).

.control {
   ...
   & + .control {
      ...
   }
}

See

https://github.com/Financial-Times/x-dash/blob/dda519f5639d0423317e3e0ec91eb0497eae0884/components/x-privacy-manager/src/components/radio-btn.scss#L13

This commit here introduced the bug 1b0c225

ah of course, missed the + bit