Skip to content

Conversation

liamdebeasi
Copy link
Contributor

@liamdebeasi liamdebeasi commented Feb 29, 2024

Issue number: Internal


What is the current behavior?

As part of my work for removing the deprecated ion-item features, I updated a selector for slotted notes. This caused the following visual diff:

Heatmap Actual Expected
image image image

This is happening because of the lines below the selector:

- ::slotted(ion-note[slot]:not([slot="helper"]):not([slot="error"])) {
+ ::slotted(ion-note[slot]) {
  @include padding($item-md-note-slot-padding-top, $item-md-note-slot-padding-end, $item-md-note-slot-padding-bottom, $item-md-note-slot-padding-start);
}

::slotted(ion-note[slot="start"]) {
  @include padding-horizontal($item-md-note-start-slot-padding-start, $item-md-note-start-slot-padding-end);
}
::slotted(ion-note[slot="end"]) {
  @include padding-horizontal($item-md-note-end-slot-padding-start, $item-md-note-end-slot-padding-end);
}

Previously, ::slotted(ion-note[slot]:not([slot="helper"]):not([slot="error"])) won out over the styles because it had a higher specificity, and the left/right padding was always 0.

When the selector changed to ::slotted(ion-note[slot]) that caused the selector to have the same specificity as the other selectors. Since the other selectors were defined later, those styles began to win out. I initially thought that fixing this was a matter of updating the specificity.

However, when doing a git blame I learned that the new selector (::slotted(ion-note[slot])) was what was there originally and the selector was changed in feature. This work was done during a time where we did not have great screenshot test coverage, so it's likely that this diff was not caught.


We also discovered that the margin on slotted content was larger than it needed to be. With this change, slotted notes had a padding of 16px in addition to the slot itself having a margin of 32px. After comparing with the MD2 spec, this behavior is incorrect. The space between the slot and the content should always be 16px:

image

What is the new behavior?

  • In order to correct this, the decided to align with the MD2 spec. To achieve this, I made the following changes:
  1. Removed any left/right padding within the slotted note
  2. Reduced the slot margin from 32px to 16px.

Does this introduce a breaking change?

  • Yes
  • No

Other information

@github-actions github-actions bot added the package: core @ionic/core package label Feb 29, 2024
@liamdebeasi liamdebeasi changed the title fix(item, item-divider): slotted note spacing is correct fix(item, item-divider): slotted spacing is correct Feb 29, 2024
@liamdebeasi liamdebeasi marked this pull request as ready for review February 29, 2024 22:05
@liamdebeasi liamdebeasi requested review from a team and brandyscarney as code owners February 29, 2024 22:05
@liamdebeasi liamdebeasi merged commit ac72531 into feature-8.0 Feb 29, 2024
@liamdebeasi liamdebeasi deleted the md-slot-margin branch February 29, 2024 22:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

package: core @ionic/core package

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants