Skip to content

Conversation

CrossEye
Copy link
Contributor

@CrossEye CrossEye commented Sep 8, 2025

See the Talk discussion on Prerelease animation for more background.

This replaces a number of post-animations blocks that set an element's CSS transition property to none with ones that remove that style property altogether. If fixes an issue in which, after an element has been animated into the story river, :hover transitions stop working.

Although @saqimtiaz suggested looking also at /core/modules/utils/dom/modal.js and at core/modules/utils/fakedom.js, I didn't see any necessary changes there. I did find two others, slide.js and cecily.js.

Testing

Do we have any automated testing feature that would help demonstrate the fix? I don't know that I've ever seen a programmatic way to assert that a CSS animation runs on precipitating event. Has anyone else?

One simple question

Should we add a function so that these lines would be replaced with $tw.utils.removeStyle(targetElement, "transition") (or possibly removeStyles using an array of names)?

A harder question

Should we take this time to review all such JS animation resets and replace all the $tw.util.setStyle calls with a property removal instead? There is a good argument to be make for doing so, but it is a substantial amount of work. And it seems as if testing would be quite difficult. Even recognizing when we're resetting to defaults rather than to arbitrary values might be difficult.

Copy link

netlify bot commented Sep 8, 2025

Deploy Preview for tiddlywiki-previews ready!

Name Link
🔨 Latest commit e40207a
🔍 Latest deploy log https://app.netlify.com/projects/tiddlywiki-previews/deploys/68bf6c028514fb000897506a
😎 Deploy Preview https://deploy-preview-9284--tiddlywiki-previews.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

Copy link

github-actions bot commented Sep 8, 2025

Confirmed: CrossEye has already signed the Contributor License Agreement (see contributing.md)

Copy link

github-actions bot commented Sep 8, 2025

📊 Build Size Comparison: empty.html

Branch Size
Base (master) 2537.6 KB
PR 2538.7 KB

Diff: ⬆️ Increase: +1.1 KB

@pmario
Copy link
Member

pmario commented Sep 8, 2025

There seem to be some problems with the pop and zoomin storyview

  • pop seems not to work as intended
  • and zoomin raises an RSOD - ReferenceError: stargetElement is not defined
    there seems to be a typo. stargetElement

Tested with FF on ubuntu 24.04

@CrossEye
Copy link
Contributor Author

CrossEye commented Sep 8, 2025

I've addressed the comments made so far, removing slide and cecily. I also did add helper functions for these changes, necessary as my adjusted approach is removing many more properties than transition, usually many at once.

I think I have pop and zoomin working correctly. pop wasn't too hard to fix, but zoomin took a little more work. In fact, the existing "Hire Jeremy" tiddler wasn't animating properly in zoomin at all. It seems to be working, but I've never done much with anything but classic, so I would appreciate a look from someone who has used the others more.

@pmario
Copy link
Member

pmario commented Sep 9, 2025

Works now for me on FF and Edge - Windows 11

@CrossEye CrossEye changed the title WIP: Remove css props rather than setting transition to none Remove css props rather than setting transition to none Sep 10, 2025
@CrossEye
Copy link
Contributor Author

Removing the "WIP:" prefix. This is ready to commit, if the team is interested. But I do think we should investigate this more widely. The more we use CSS animations, the less we want hard-coded element style properties staying around when the element is at rest. I think we might want to look at where else we might use such techniques.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants