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

feat: Editorial review: Add content for overlay and discrete animation features #29943

Merged
merged 36 commits into from
Nov 20, 2023

Conversation

chrisdavidmills
Copy link
Contributor

@chrisdavidmills chrisdavidmills commented Oct 31, 2023

Description

This PR is for the editorial review of #29049, which was kindly tech-reviewed by @josepharhar.


Description

This PR adds content for multiple related features that go together to enable the creation of exit/entrance animations using simple CSS, or to put it another way, animating to/from display: none (this includes top layer elements like popovers), or animating elements when they have just been added to the DOM.

Specifically, these are:

Motivation

Additional details

Related issues and pull requests

@chrisdavidmills chrisdavidmills requested review from a team as code owners October 31, 2023 17:07
@chrisdavidmills chrisdavidmills requested review from wbamberg, estelle and bsmth and removed request for a team October 31, 2023 17:07
@github-actions github-actions bot added Content:CSS Cascading Style Sheets docs Content:HTML Hypertext Markup Language docs Content:WebAPI Web API docs labels Oct 31, 2023
files/en-us/web/api/popover_api/using/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/popover_api/using/index.md Outdated Show resolved Hide resolved
files/en-us/web/css/@starting-style/index.md Outdated Show resolved Hide resolved
files/en-us/web/css/content-visibility/index.md Outdated Show resolved Hide resolved
@dipikabh

This comment was marked as resolved.

@chrisdavidmills

This comment was marked as resolved.

estelle

This comment was marked as resolved.

files/en-us/web/api/popover_api/using/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/popover_api/using/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/popover_api/using/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/popover_api/using/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/popover_api/using/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/popover_api/using/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/popover_api/using/index.md Outdated Show resolved Hide resolved
files/en-us/web/api/popover_api/using/index.md Outdated Show resolved Hide resolved
Comment on lines 115 to 121
transition:
opacity 0.7s,
transform 0.7s,
overlay 0.7s allow-discrete,
display 0.7s allow-discrete;
/* Equivalent to
transition: all 0.7s allow-discrete; */
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
transition:
opacity 0.7s,
transform 0.7s,
overlay 0.7s allow-discrete,
display 0.7s allow-discrete;
/* Equivalent to
transition: all 0.7s allow-discrete; */
transition-property: opacity, transform, overlay, display;
transition-duration: 0.7s;
transition-behavior: allow-discrete;
/* Using the shorthand `transition` property, we would write:
transition:
opacity 0.7s,
transform 0.7s,
overlay 0.7s allow-discrete,
display 0.7s allow-discrete;
or, even:
transform: 0.7s allow-discrete;
*/

i have to insist that the property be used in the examples

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, thanks. I'm fine with this suggestion as we are showing the shorthand equivalents in the comment. I've mostly used your suggestion, but with a bit of tweaking to the prose; I've also fixed the ultra-compact shorthand suggestion.

@@ -49,7 +49,7 @@ This example demonstrates the create a non-modal dialog by using only HTML. Beca

#### Result

{{EmbedLiveSample("Caveats_of_creating_a_dialog_using_only_HTML", "100%", 200)}}
{{EmbedLiveSample("HTML-only_dialog", "100%", 200)}}
Copy link
Member

Choose a reason for hiding this comment

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

There links to these anchors need to be updated

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've grepped through the repo and can't find anywhere that links to this. Have you found a particular instance that is wrong?

Copy link
Member

Choose a reason for hiding this comment

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

line 192 (and the other link is a few lines lower) on this page

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, sorry about that; I managed to totally miss it. Fixed now.

- [`@starting-style`](/en-US/docs/Web/CSS/@starting-style) at-rule
- : Provides a set of starting values for properties set on the `<dialog>` that you want to transition from when it is first shown. This is needed to avoid unexpected behavior. By default, CSS transitions only occur when a property changes from one value to another on a visible element; they are not triggered on elements' first style updates, or when the `display` type changes from `none` to another type.
- [`display`](/en-US/docs/Web/CSS/display) property
- : Add `display` to the transitions list so that the `<dialog>` will remain as `display: block` for the duration of the transition, ensuring the other transitions are visible.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- : Add `display` to the transitions list so that the `<dialog>` will remain as `display: block` for the duration of the transition, ensuring the other transitions are visible.
- : Add `display` to the transitions list so that the `<dialog>` will remain as `display: block` (or other visible `display` value set on the dialog's open state) for the duration of the transition, ensuring the other transitions are visible.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added

When animating `<dialog>`s with CSS transitions, the following features are required:

- [`@starting-style`](/en-US/docs/Web/CSS/@starting-style) at-rule
- : Provides a set of starting values for properties set on the `<dialog>` that you want to transition from when it is first shown. This is needed to avoid unexpected behavior. By default, CSS transitions only occur when a property changes from one value to another on a visible element; they are not triggered on elements' first style updates, or when the `display` type changes from `none` to another type.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- : Provides a set of starting values for properties set on the `<dialog>` that you want to transition from when it is first shown. This is needed to avoid unexpected behavior. By default, CSS transitions only occur when a property changes from one value to another on a visible element; they are not triggered on elements' first style updates, or when the `display` type changes from `none` to another type.
- : Provides a set of starting values for properties set on the `<dialog>` that you want to transition from every time the dialog is opened. This is needed to avoid unexpected behavior. By default, CSS transitions only occur when a property changes from one value to another on a visible element; they are not triggered on elements' first style updates, or when the `display` type changes from `none` to another type.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added

Comment on lines 301 to 305

body,
button {
font-family: system-ui;
}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
body,
button {
font-family: system-ui;
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure; removed.


##### CSS

In the CSS, we include a `@starting-style` block that defines the transition starting styles for the `opacity` and `transform` properties, transition end styles on the `dialog[open]` state, and default styles on the default `dialog` state to transition back to once the `<dialog>` has appeared. Note how the the `<dialog>`'s `transition` list includes not only these properties, but also the `display` and `overlay` properties, each with `allow-discrete` set on them.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
In the CSS, we include a `@starting-style` block that defines the transition starting styles for the `opacity` and `transform` properties, transition end styles on the `dialog[open]` state, and default styles on the default `dialog` state to transition back to once the `<dialog>` has appeared. Note how the the `<dialog>`'s `transition` list includes not only these properties, but also the `display` and `overlay` properties, each with `allow-discrete` set on them.
In the CSS, we include a `@starting-style` block that defines the transition starting styles for the `opacity` and `transform` properties, transition end styles on the `dialog[open]` state, and default styles on the default `dialog` state to transition back to once the `<dialog>` has appeared. Note how the `<dialog>`'s `transition` list includes not only these properties, but also the `display` and `overlay` properties, each with `allow-discrete` set on them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed; thanks!

}
```

Note the `@starting-style` block used to specify the starting style for the transition, and the inclusion of the `display` property in the trainsitions list, with `allow-discrete` set on it.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Note the `@starting-style` block used to specify the starting style for the transition, and the inclusion of the `display` property in the trainsitions list, with `allow-discrete` set on it.
Note the `@starting-style` block used to specify the starting style for the transition, and the inclusion of the `display` property in the transitions list, with `allow-discrete` set on it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed


The code renders as follows:

{{ EmbedLiveSample("Animating a popover", "100%", "200") }}
Copy link
Contributor

@OnkarRuikar OnkarRuikar Nov 16, 2023

Choose a reason for hiding this comment

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

Suggested change
{{ EmbedLiveSample("Animating a popover", "100%", "200") }}
{{ EmbedLiveSample("Transitioning a popover", "100%", "200") }}

Here section heading doesn't match. Even though it works now but may fail mysteriously in future.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, good catch; thanks! Fixed in next commit.


The code renders as follows:

{{ EmbedLiveSample("Animating a popover", "100%", "200") }}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
{{ EmbedLiveSample("Animating a popover", "100%", "200") }}
{{ EmbedLiveSample("Transitioning a popover", "100%", "200") }}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed ;-)

transition-property: font-size;
transition-duration: 4s;
transition-property: background-color;
transition-duration: 0.7s;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
transition-duration: 0.7s;
transition-duration: 1s;

Pros says it undergoes a one-second color transition.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated; thanks

@@ -49,7 +49,7 @@ This example demonstrates the create a non-modal dialog by using only HTML. Beca

#### Result

{{EmbedLiveSample("Caveats_of_creating_a_dialog_using_only_HTML", "100%", 200)}}
{{EmbedLiveSample("HTML-only_dialog", "100%", 200)}}
Copy link
Contributor

Choose a reason for hiding this comment

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

This example doesn't work in playground.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK; can you file an issue to look at this separately? This is really a playground issue rather than an issue with my content update.

@@ -49,7 +49,7 @@ This example demonstrates the create a non-modal dialog by using only HTML. Beca

#### Result

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
> **Note:** Reload page to reset the output.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added

We also set a starting style value for the {{cssxref("background-color")}} property on the [`::backdrop`](/en-US/docs/Web/CSS/::backdrop) that appears behind the `<dialog>` when it opens, to provide a nice darkening animation. The `dialog[open]::backdrop` selector selects only the backdrops of `<dialog>` elements when the dialog is open.

```css
/* Open state of the dialog */
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
/* Open state of the dialog */
/* Open state of the dialog */

To match other two spaced comments.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

@@ -185,6 +185,107 @@ This CSS establishes the look of the menu, with the background and text colors b

{{EmbedLiveSample("Using transitions when highlighting menus")}}

### Transitioning display and content-visibility

This example demonstrates how [`display`](/en-US/docs/Web/CSS/display) and [`content-visibility`](/en-US/docs/Web/CSS/content-visibility) can be transitioned. This behavior is useful for creating entry/exit animations where you want to for example remove a container from the DOM with `display: none`, but have it fade out with [`opacity`](/en-US/docs/Web/CSS/opacity) rather than disappearing immediately.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
This example demonstrates how [`display`](/en-US/docs/Web/CSS/display) and [`content-visibility`](/en-US/docs/Web/CSS/content-visibility) can be transitioned. This behavior is useful for creating entry/exit animations where you want to for example remove a container from the DOM with `display: none`, but have it fade out with [`opacity`](/en-US/docs/Web/CSS/opacity) rather than disappearing immediately.
This example demonstrates how [`display`](/en-US/docs/Web/CSS/display) and [`content-visibility`](/en-US/docs/Web/CSS/content-visibility) can be transitioned. This behavior is useful for creating entry/exit animations where you want to for example remove a container from the DOM with `display: none`, but have it fade out with [`opacity`](/en-US/docs/Web/CSS/opacity), or some other exit animation, rather than disappearing immediately.

optional

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think this is necessary; the text already says this is an example, rather than what always happens.


When transitioning these properties, two additional features are also needed:

- [`@starting-style`](/en-US/docs/Web/CSS/@starting-style) is used to provide a set of starting values for properties set on an element that you want to transition from when the element receives its first style update. This is needed to avoid unexpected behavior. By default, CSS transitions are not triggered on elements' first style updates, or when `display`/`content-visibility` changes from `none`/`hidden` to another state.
Copy link
Member

Choose a reason for hiding this comment

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

Every time you exit and then animate from display: none; the start style is used, because it is removed and put back into the dom. This covers that. With content-visibility, when it is animated the second time, does it start from start-styles each time? Or from the default state? If these two properties are different in this way, we should note that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Heh, I'm really glad you brought this up. I tested content-visibility, and it turns out it doesn't need @starting-styles; content-visibility doesn't actually remove the content from the DOM. It transitions from the default state styles each time. You can see proof here:

https://content-visibility-transition-test.glitch.me/

(The source Glitch is at https://glitch.com/edit/#!/content-visibility-transition-test?path=index.html%3A36%3A8, in case you want to remix/play with it).

I'll make the necessary changes to all pages now, in a separate commit.

Copy link
Member

@estelle estelle left a comment

Choose a reason for hiding this comment

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

review of transition behavior: I couldn't create a nested code comment, so my comment is the suggested edit

Comment on lines 63 to 77
### Basic usage

```css
.card {
transition-property: opacity, display;
transition-duration: 0.25s;
transition-behavior: allow-discrete;
}

.card.fade-out {
opacity: 0;
display: none;
}
```

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
### Basic usage
```css
.card {
transition-property: opacity, display;
transition-duration: 0.25s;
transition-behavior: allow-discrete;
}
.card.fade-out {
opacity: 0;
display: none;
}
```

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, I see. So you are suggesting not putting this as an example, and moving it to the description section. Yup, sounds reasonable. Done.

When attempting to transition properties with a [discrete animation type](/en-US/docs/Web/CSS/CSS_animated_properties#discrete), `transition-behavior` must be set to `allow-discrete` to enable those transitions to occur.

This is most significant in the cases of [`display`](/en-US/docs/Web/CSS/display), [`content-visibility`](/en-US/docs/Web/CSS/display), and [`overlay`](/en-US/docs/Web/CSS/overlay), which historically were not animatable. The ability of these elements to be transitioned means that it is fairly easy to create entry and exit transitions, where an element is transitioned to and from a hidden state (which includes elements appearing in the [top layer](/en-US/docs/Glossary/Top_layer) such as [popovers](/en-US/docs/Web/API/Popover_API) or modal {{htmlelement("dialog")}} elements), or transitioned as soon as it is added to the DOM.

Copy link
Member

@estelle estelle Nov 16, 2023

Choose a reason for hiding this comment

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

This is my suggestion, but Github doesn't like nesting code in suggestions:

The transition-behavior property is only relevant when used in conjunction with other transition properties, notably {{cssxref("transition-property")}} and {{cssxref("transition-duration")}}, as no transition occurs if no properties are animated over a non-zero duration of time.

.card {
  transition-property: opacity, display;
  transition-duration: 0.25s;
  transition-behavior: allow-discrete;
}

.card.fade-out {
  opacity: 0;
  display: none;
}

The transition-behavior value can be included as part of a shorthand {{cssxref("transition")}} declaration. When included in the shorthand, when using or defaulting to all properties, theallow-discrete has no impact on regular animatable properties. The following CSS is equivalent to the longhand declarations above:

.card {
  transition: all 0.25s;
  transition: all  0.25s allow-discrete;
}

.card.fade-out {
  opacity: 0;
  display: none;
}

We included the transition property twice. We included the transition without the allow-discrete value before the one with the value to ensure the card's other properties still transition if a browser doesn't support transition-behavior.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good; added, with a few tweaks.

When animating `display` with [CSS transitions](/en-US/docs/Web/CSS/CSS_transitions), two additional features are needed:

- [`@starting-style`](/en-US/docs/Web/CSS/@starting-style) provides starting values for properties you want to transition from when the animated element is first shown. This is needed to avoid unexpected behavior. By default, CSS transitions are not triggered on an element's first style update or when the `display` type changes from `none` to another type.
- [`transition-behavior: allow-discrete`](/en-US/docs/Web/CSS/transition-behavior) needs to be set on `display` when it is transitioned to enable `display` transitions.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- [`transition-behavior: allow-discrete`](/en-US/docs/Web/CSS/transition-behavior) needs to be set on `display` when it is transitioned to enable `display` transitions.
- [`transition-behavior: allow-discrete`](/en-US/docs/Web/CSS/transition-behavior) needs to be set on the {{cssxref("transition-property")}} declaration (or the {{cssxref("transition")}} shorthand) to enable `display` transitions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

```

#### CSS

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
The `overlay` property is only present in list of transitioned properties. As `overlay` is a user-agent controlled property, it is neither declared in the pre-transition or post-transition state.

We aren't including the overlay property as a property. We need to explain why. This is my attempt. You might have a better though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added


{{CSSRef}}{{SeeCompatTable}}

The **`overlay`** [CSS](/en-US/docs/Web/CSS) property specifies whether an element appearing in the [top layer](/en-US/docs/Glossary/Top_layer) (for example, a shown [popover](/en-US/docs/Web/API/Popover_API) or modal {{htmlelement("dialog")}} element) is actually rendered in the top layer.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
The **`overlay`** [CSS](/en-US/docs/Web/CSS) property specifies whether an element appearing in the [top layer](/en-US/docs/Glossary/Top_layer) (for example, a shown [popover](/en-US/docs/Web/API/Popover_API) or modal {{htmlelement("dialog")}} element) is actually rendered in the top layer.
The **`overlay`** [CSS](/en-US/docs/Web/CSS) property specifies whether an element appearing in the [top layer](/en-US/docs/Glossary/Top_layer) (for example, a shown [popover](/en-US/docs/Web/API/Popover_API) or modal {{htmlelement("dialog")}} element) is actually rendered in the top layer. This property is only relevant within a list of {{cssxref("transition-property")}} values, and only if `allow-discrete` is set as the {{cssxref("transition-behavior")}}.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added


## See also

- [CSS transitions](/en-US/docs/Web/CSS/CSS_transitions)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- [CSS transitions](/en-US/docs/Web/CSS/CSS_transitions)
- [CSS transitions](/en-US/docs/Web/CSS/CSS_transitions) module

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added

Copy link
Member

@estelle estelle left a comment

Choose a reason for hiding this comment

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

content-visiblility page: Just an h3 title edit for an example (and the same edit in the rendering function call).

@@ -149,6 +169,106 @@ document.querySelectorAll("button.toggle").forEach((button) => {

{{ EmbedLiveSample('Using hidden to manually manage visibility') }}

### content-visibility animation example
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
### content-visibility animation example
### Animating content-visibility

it's in examples, so we don't need to add that to the header

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call. I've changed this heading, and all the links/references to it.


The rendered result looks like this:

{{ EmbedLiveSample("content-visibility animation example", "100%", "300") }}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
{{ EmbedLiveSample("content-visibility animation example", "100%", "300") }}
{{ EmbedLiveSample("Animating content-visibility", "100%", "300") }}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

changed

@estelle
Copy link
Member

estelle commented Nov 17, 2023

see #29943 (comment)

Copy link
Member

@estelle estelle left a comment

Choose a reason for hiding this comment

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

Squeeeeeeeeeeeee! 🎉

@estelle estelle merged commit bed59f2 into mdn:main Nov 20, 2023
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content:CSS Cascading Style Sheets docs Content:HTML Hypertext Markup Language docs Content:WebAPI Web API docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants