Animation: Fade out unmounted thought

[zukilover]

#2526

This update implements the use of CSSTransition to apply the .fade CSS classes when items are unmounted.

Please note that the current implementation uses the existing .fade classes, which have a transition duration of 200ms, while the timeout for the transition is set to veryFastDuration (80ms).

em/src/App.css

Lines 21 to 24 in 9e7f8a1

	.fade-exit-active {
	opacity: 0;
	transition: opacity 0.2s ease-out;
	}

Let me know if you'd prefer to create a new CSS class to match the durations more closely.

[raineorshine]

I tested and can confirm the new unmount animation. Looks good!

I added a very-fast-fade animation class to match the veryFastDuration.

This PR effectively adds both a fade-in and fade-out animation via the CSSTransition. However, thoughts already faded in on mount. This makes me think that we are applying a redundant fade-in animation somewhere else. This will have an impact on performance, and it's probably better from a code design perspective to avoid redundant animations. Can you find the old fade-in animation and suggest a way to eliminate the redundancy?

Lastly, we should confirm the easing function and duration of the old fade-in animation and make sure that is preserved.

Thanks!

[zukilover]

The existing fade animation likely come from these lines:

em/src/components/Subthought.tsx

Lines 127 to 130 in 560ff54

	opacity: thought.value === '' ? opacity : '0',
	transition: autofocusChanged
	? `opacity {durations.layoutSlowShiftDuration} ease-out`
	: `opacity {durations.layoutNodeAnimationDuration} ease-in`,

I will check further.

Update: This line has nothing todo with the thoughts being faded in on mount.

[zukilover]

@raineorshine I reviewed the line here:

em/src/components/Subthought.tsx

Lines 127 to 130 in 560ff54

	opacity: thought.value === '' ? opacity : '0',
	transition: autofocusChanged
	? `opacity {durations.layoutSlowShiftDuration} ease-out`
	: `opacity {durations.layoutNodeAnimationDuration} ease-in`,

The thought does indeed fade in on mount, but this effect is immediately overridden by the tree-node's very-fast-fade-enter animation when the component mounts. In other words, it shouldn't impact the initial mount behavior on the first render.

Additionally, the specific line mentioned controls the opacity when the autofocus changes on a thought. Please let me know if this aligns with your understanding.

[zukilover]

@raineorshine the Ancestors snap in on cursorUp issue is fixed by refactoring the autofocus transition now that we use FadeTransition. From my latest check, it is working as expected.

I will double check to make sure.

[zukilover]

I have checked the latest update, here's how it works:

screencast-localhost_3000-2024_11_26-12_39_48.webm

We can see from individual frames that the opacity works as expected on autofocus:

[raineorshine]

Unfortunately that's not the expected behavior. Thoughts should not snap to full opacity as soon as the cursor moves to them.

Take a closer look at the difference between Current and Expected shown in 2. Ancestors snap in on cursorUp: #2581 (review).

[zukilover]

@raineorshine I think the opacity inline style within VirtualThought need to be refactored into fadeTransition. Will @yangchristina be able to help with this? I've been struggling with it quite some time, but couldn't come up with the correct setup.

I mapped opacities into these transitions:

nodeAutofocusHideOpacity: {
        enter: { transition: `opacity {durations.layoutSlowShiftDuration} ease-out` },
        enterActive: { opacity: 0, transition: `opacity {durations.layoutSlowShiftDuration} ease-out` },
        enterDone: { opacity: 0, transition: `opacity {durations.layoutSlowShiftDuration} ease-out` },
        exit: { opacity: 0 },
        exitActive: { transition: `opacity {durations.layoutSlowShiftDuration} ease-out` },
      },
      nodeAutofocusDimOpacity: {
        enter: { opacity: 0.5, transition: `opacity {durations.layoutSlowShiftDuration} ease-out` },
        enterActive: { opacity: 0.5, transition: `opacity {durations.layoutSlowShiftDuration} ease-out` },
        enterDone: { opacity: 0.5, transition: `opacity {durations.layoutSlowShiftDuration} ease-out` },
        exit: { opacity: 0.5 },
        exitActive: { transition: `opacity {durations.layoutSlowShiftDuration} ease-out` },
      },
      nodeAutofocusShowOpacity: {
        enter: { opacity: 0.5 },
        enterActive: { opacity: 1, transition: `opacity {durations.layoutSlowShiftDuration} ease-out` },
        enterDone: { opacity: 1, transition: `opacity {durations.layoutSlowShiftDuration} ease-out` },
        exitActive: { transition: `opacity {durations.layoutSlowShiftDuration} ease-out` },
      },

but still no luck.

[raineorshine]

FadeTransition is used to fade in/out elements on mount/unmount. Autofocus opacity is applied to thoughts that are already mounted, so FadeTransition doesn't seem like a good fit here.

It seems like the FadeTransition opacity is overriding the autofocus opacity, but we want it the other way around. Maybe opacity: 1 does not need to be applied during some phases of the animation? Unfortunately I'm not able to get too much deeper into the technical details, as I have a lot of other responsibilities at the moment.

I would suggest backtracking to the last point where autofocus was working. Building on a broken autofocus is probably not a good idea. Then find out how to animate on unmount from there.

[zukilover]

Hi @raineorshine, I've pushed an update that separates the FadeTransition for the thought while preserving the existing autofocus behavior.

This update addresses the three issues we discussed earlier:

1. Ensures the thought fades out when unmounted.
2. Keeps the autofocus behavior unchanged.
3. Synchronizes the fade-in transition with the layoutNodeAnimation by introducing a new nodeOpacity duration.

[raineorshine]

Thanks so much! I'll let @trevinhofmann do the first review.

[trevinhofmann]

Thank you, @zukilover! The latest implementation looks quite clean and simple, and everything appears to be behaving as expected.

Both fade in and fade out are functioning as I would expect:

Screen.Recording.2024-12-04.at.9.54.55.AM.mov

The "Ancestors snap in on cursorUp" issue is also resolved:

Screen.Recording.2024-12-04.at.9.57.37.AM.mov

[raineorshine]

Thanks! This is looking very good.

Here is what I'm noticing.

3. Empty thought fades in

There used to be a special case for the empty thought so it would snap in rather than animate in. This was lost in the new implementation.

Current Behavior

newthought-current.mov

Expected Behavior

newthought-expected.mov

4. Long fade-in

Steps to Reproduce

- a
  - b
- c

Use Ctrl/Cmd + ArrowUp and Ctrl/Cmd + ArrowDown to move the cursor between a and c.

Current Behavior

Notice how b takes longer than expected (at least 500ms) to reach full opacity.

fadein-current.mov

Expected Behavior

fadein-expected.mov

Notes

I'm not sure why the fade-in exceeds the nodeOpacity duration. It seems as if it is fading in with the layoutSlowShift duration which is what the Subthought autofocus opacity transition uses. But the Subthought component has not changed at all, so I'm not sure how that could be implicated.

Could it be that the thought is not being unmounted, and thus the [autofocusChanged] ref is not being renewed as expected? Should we being setting unmountOnExit on the FadeTransition?

5. Layered fade-in

This is more speculative, as I don't have a clearly observable test case, but it may currently be masked by #4 above.

If the goal is to only add a fade-out animation, why apply a fade-in animation with FadeTransition at all? CSSTransition can be configured to only animate on exit. This relates to an older comment I made about redundant fade-in animations. It seems that layering fade-in animations may lead to unexpected behavior. (Not to mention composing easing functions makes it harder to design animation timing.)

[zukilover]

@raineorshine you are correct, we shouldn't touch mounting thought animation. I've pushed updates so that it focuses on the exit transition. I've also added unmountOnExit that should fix the long fade-in issue. Kindly re-review.

[raineorshine]

Thanks, looks great now! That totally took care of issues 3,4,5.

I discovered an error when navigating a larger thoughtspace. Confirmed that this does not occur in main.

Steps to Reproduce

I'm sorry I wasn't able to reproduce it with a more minimal test case.

- Buddhist Art
  - Bodhisattvas
    - Avalokiteshvara
      - “One who who hears the cries of the world”
      - Korean: Guanyin
      - Japanese: Kannon
      - Often has a small Amithaba figure in the crown
      - Compassion
      - Lotus bloom
      - Medicine flask
      - Wish-fulfilling jewel
        - Possibly develops in Paekche
      - Eleven heads
        - Given by Amithaba to help tend to all those who suffer
      - Usually the center of a mandorla triad
    - Green Tara
      - Abundance
      - Auspiciousness
      - Action/Force
    - Kshitigarbha
      - Japanese: Jizo
      - Korean: Jijang
      - Monk
      - Close-cropped hair
      - Staff
    - Manjushri
      - Japanese: Monju
      - Sword and noose
        - Cuts through ignorance
      - Prajna
        - Wisdom
        - Insight
      - Rides a blue lion
      - Resides in the eastern Pure Land
    - Samantabhadra
      - “Universal Worthy”
      - Japanese: Kugen
      - 12th–14th c.
        - Heian period
      - Connected to the Lotus Sutra
      - Six-tusked white elephant
      - Often serves as Shakyamuni’s attendant with:
        - Manjushri
      - Worshipped independently in Japan
      - Tendai school
    - Vajrapani
      - “vajra holder”
      - Power
    - Vajrasattva
      - Esoteric form of Vajrapani
  - Buddhas
    - Akshoba
      - Presides over the cementerial
    - Amithaba
      - Korean: Amita
      - Pure land Buddhism
      - Buddha of Infinite Light
      - Fingers curled and knuckles pressed together
    - Bhaishajyaguru
      - Buddha of Healing
    - Dipankara
      - Past Buddha that has been forgotten
    - Maitreya
      - Future Buddha
      - Standing on a lotus base instead of sitting suggests that Maitreya resides in heaving, awaiting his final rebirth
      - Water flask (globe) of an ascetic
      - Pensive pose
        - hanka shii
      - Originated with images of Shakyamuni as Prince
      - Emerged with Pure Land Buddhism during a time of war social chaos
        - Northern Wei split violently into Eastern and Western Wei
    - Shakyamuni
      - Double bun
        - Aristocratic East Asian youth, 6th c.
    - Vairochana
      - Formed from the mind-made buddha created upon enlightenment
      - Jewels and crown representing Vairochana as a living Buddha presiding over the heavens
      - Conceptual map of the universe
      - Sometimes represented by:
        - Bumhisparsha
  - Development
    - *Alternative structure*
    - 1st–2nd c. Anthropomorphic representations
    - 3rd c. — China
    - 4–5th c. — Korea
      - Introduced through foreign diplomatic relations
      - Aristocratic class only
    - 5–6th c. — Japan
    - 7–9th c. — Vajrayana → Japan
    - 6–7th c. — Vajrayana → S.E. Asia
      - Maritime routes
    - 11th c. — Himalayas
    - 11th–13th c. — Mahayana/Vajrayana spread
  - Episodes/Motifs
    - Parinirvana
      - Final nirvana
      - Late 11th c. — Lying on his back
      - Early 14th c. — Lying on side with right arm as pillow
  - Medium
    - Dry lacquer
      - Few extant examples
      - Clay core with layers of fabric strips coated in resin (lacquer)
      - Multiple layers of lacquer with thickened including oil and burned bone
  - Mudras
    - Abhaya Mudra
      - Hand raised
      - Approachability
    - Bumhisparsha
      - Touching the earth
      - Moment of enlightenment
    - Vitarka
      - Thumb and index finger touching, palm out
      - Teaching
      - Exposition
    - Varada
      - Palm out, fingers down
      - Granting boons
      - Wish granting
    - Dhyana
      - Meditation
      - One hand covering the other in front of hara
      - Does not occur in Japan?
    - Sovereignty
      - Right arm raised, left arm lowered
      - Newborn Buddha
      - Supremacy over all that exists above or below the heavens
    - Wisdom-fist
      - Two stacked fists
      - Mahavairocana
        - Esoteric Buddhism
      - Supreme Buddha of the Avatamsaka teaching
        - Korea
  - Poses & Motifs
    - King Aśoka Buddha
      - Northern Wei → Koguryo → Japan
      - Japan
        - More solemn face
        - Shawl crosses lower near the ankles
        - More often holds wish-fulfilling jewel than the “heart-shaped attribute”
          - Paekche
        - High crown
          - Paekche
    - Hanka shii
      - 7th c. Japan
      - Pensive
      - hanka
        - “Legs half-crossed”
      - shii
        - Meditation
        - Finger gently touching the cheek
      - Shou-cordon
        - Decorative cloth hanging from the waist
        - Fashioned after Chinese court attire
        - Jade ring
        - Tucked under the figure’s bottom, bottom braids separate from top
          - Silla
        - Hanging straight down, too and bottom braids are connected
          - Paekche
        - Derived from:
          - Northern Qi
          - Sui
          - Early Tang
      - Origin
        - India
        - Originally represented Shakyamuni and then Maitreya
    - Single mandorla triad
      - Distinctly Korean
        - Scrolling vine and concentric circles within head halo
        - Crest of the head halo
          - Wish-fulfilling jewels
        - Head halo light ray pattern
        - Attendants extend beyond the edge of the mandorla
        - Interval petals
          - Artichoke style
        - Space between feet
        - Placement of Transformation Buddhas
        - Eyebrows chiseled after casting
      - vs Chinese
        - “Snake-dragon” flame pattern
        - Wider halos
        - Attendants are smaller and more distant from the central Buddha
      - Influenced by:
        - Northern Wei
    - Oversized head

1. Set the cursor on Buddhist Art
2. Set the cursor on Bodhisattvas
3. Set the cursor on Avalokiteshvara

Current Behavior

Error:

Rendered fewer hooks than expected

Expected Behavior

No error.

[zukilover]

@raineorshine I have fixed the Rendered fewer hooks than expected issue by moving the useRef hook to the top of the render logic.

[raineorshine]

Great, thanks!

Just awaiting your reply here: #2581 (comment)

[zukilover]

@raineorshine Yes, to avoid the enter animation, I've updated the fadeTransition to include the full opacity at enter.

[raineorshine]

Thanks for your persistence. Everything is working great!