Skip to content
This repository has been archived by the owner on Jun 30, 2018. It is now read-only.

Added definition for Animation #727

Closed
wants to merge 28 commits into from
Closed

Added definition for Animation #727

wants to merge 28 commits into from

Conversation

alastc
Copy link
Contributor

@alastc alastc commented Jan 14, 2018

Based on comment #697. Not sure if it is ok to change the first word of SC text "Animations" to the singluar to match the definition?

@alastc
Copy link
Contributor Author

alastc commented Jan 14, 2018

Hmm, not sure why there is so much in this merge request, the two files I edited are the SC and adding a new definition. The rest is probably from merging in changes from the W3C repo, including master.

@awkawk
Copy link
Member

awkawk commented Jan 14, 2018

@alastc Did you make a new branch off Master before making the changes or did you start from the "animation-from-interaction" branch that was already present?

@awkawk
Copy link
Member

awkawk commented Jan 14, 2018

Also:

  1. Please leave the SC text as it was, that is what the data-lt attribute in the definition is for
  2. The definition seems to be for character keys

Forgot to save before pushing in the last attempt.
@alastc
Copy link
Contributor Author

alastc commented Jan 14, 2018

Ach, just seen that I was trying to merge into master, that wasn't intentional.

I haven't edited for a while so I merged all the things. (W3C-master > alastc-master, W3C-animations > alastc-animations, and just about every connotation I could find that went from W3C to my fork.

I think if we cancel this one, I'll undo the change to the SC text and do a PR to the animations branch.

Also, just seen the data-lt attribute, got-it.

@alastc alastc closed this Jan 14, 2018
@alastc
Copy link
Contributor Author

alastc commented Jan 14, 2018

Oh dear, if I merge my animations branch to the W3C one, it brings over all the changes from master, around 190, which is worse than this one! Doh, not sure how to back that out.

@alastc alastc reopened this Jan 14, 2018
@alastc
Copy link
Contributor Author

alastc commented Jan 14, 2018

Ok, done those. It also brings in a minor update to the understanding I was doing on the same branch, sorry.

Is it ok that the SC text is "Animations" but the definition is "animation", singluar?

Copy link
Member

@michael-n-cooper michael-n-cooper left a comment

Choose a reason for hiding this comment

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

The SC changes look ok to me though I haven't been following substantive discussion. There's a change to readme.md that seems like noise and appears to introduce a typo, so I'd like that removed from the pull request before merging.

As a minor issue to support future maintainability, the filename for the definition should be "animation.html" not "animations.html" since the term is defined in the singular (and generally terms should be defined in the singular even if referenced in plural, like in this example - i.e., the term itself looks good structurally, just the filename needs updating).

Actually, while I'm looking at that, the first paragraph of the term should be a clause that can be dropped in place of the term, so it should drop the initial "The" and the ending full stop.

The second paragraph should perhaps use class="example".

@alastc
Copy link
Contributor Author

alastc commented Jan 14, 2018

@michael-n-cooper I've done those apart from the readme one, that isn't just backing out, it came from merging master into my branch (trying to avoid conflicts). I'm not sure how to exclude that, I can't undo it as such.

@detlevhfischer
Copy link
Contributor

My one concern would be whether the definition addresses the point of one-step-only changes which would bring like user-initiated changes of size, added outline etc done for highlighting focus into the realm of this SC. Is that intended. I think not. I think Patrick made this point, and it seems open in this definition.

@alastc
Copy link
Contributor Author

alastc commented Jan 15, 2018

Hi Detlev,

From what we know of the vestibular triggers, those types of changes aren't a trigger, it requires a visible transition. If there is no transition, it isn't covered by this definition.

@detlevhfischer
Copy link
Contributor

detlevhfischer commented Jan 15, 2018

Ok I see that addition of steps between states implies that a third step is added. So if someone would grow an 2px focus outline via a 1 pixel step, it would technically fall under animation and likely still not cause issues. This may be nitpicking and I am certainly not pressing this point, just saying...

The one thing that seems a bit odd in the revised definition in CfC of issue 697 is the bit 'For example', because the first thing that follows is not an example for the def text. I would just drop "For example". One could actually add the common focus outline case that people may worry about for clarification, e.g.:
For example, an element which appears instantly in one frame @@@ or as a result of focusing@@@ is not using animation. This is just an idea and certainly not critical, not meant to slow things down..

@patrickhlauke
Copy link
Member

I'd still voice a slight concern that this tacitly only counts movement and/or size change as animation, without any mention of color, opacity, or other types of animation. Would be good to clarify somewhere (normatively, ideally, or non-normatively) that the scope of the SC is limited to those types of animation.

Copy link
Member

@michael-n-cooper michael-n-cooper left a comment

Choose a reason for hiding this comment

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

I should have raised this on my earlier review, but I think this pull request should be made against the working branch animation-from-interactions, not directly to the master branch. If somebody else has worked in that branch, their edits could be lost by bypassing that part of the process. If there are conflicts with that branch, they should be sorted out prior to merging to master. Also I can help fix the issue with readme in that branch, which I can't do when the pull request comes from a private repo. I need a clean merge into master to make sure problems don't emerge later, so though the readme issue seems small, it's important to repo health. Also if material goes into master that didn't come from the working branch, that branch will be seen as out of date by the repo and that might cause problems later when it comes time to clean up branches - even to causing the edits to be unintentionally reverted during a branch cleanup.

@awkawk
Copy link
Member

awkawk commented Jan 16, 2018

I have been making the edits of this sort by making a new branch off of the current Master and then making the changes. This minimizes the file collisions substantially.

@mraccess77
Copy link

@patrickhlauke. If I recall correctly we were trying to target specific types of animation such as parallax scrolling So I believe it was limited based on information as we knew that were problematic. As for the definition of animation even if there were no changes in a frame - animation between frames could be an issue but I don’t think weee wanted to tackle all of that for this round.

@alastc
Copy link
Contributor Author

alastc commented Jan 16, 2018

@patrickhlauke:

I'd still voice a slight concern that this tacitly only counts movement and/or size change as animation, without any mention of color, opacity, or other types of animation. Would be good to clarify somewhere (normatively, ideally, or non-normatively) that the scope of the SC is limited to those types of animation.

Working from comments at TPAC, the intent seemed to be that the SC is not scoped to purely movement. Although the main use-case is movement/scaling, the thought was that it would help others if it was all animation. (Remember there was a previous version that was for movement & scaling.) Also, a definition of animation should not be scoped to just movement either. Do you think the new text excludes other types of animation?

@michael-n-cooper I'll have to re-do this tonight, unless we can use Andrew's version. I made the mistake of merging W3C master into my animations branch, thinking it would be upto date and reduce conflicts. It had the opposite effect.

@patrickhlauke
Copy link
Member

Also, a definition of animation should not be scoped to just movement either. Do you think the new text excludes other types of animation?

no I think there's no evidence (unless I missed it) that animations other than motion/size animations cause problems to users with vestibular issues. so this SC would require authors to, for instance, use the reduced motion media query as a hint to avoid any kind of animation (like a subtle fade in color or similar). it would fail things that aren't actually problems.

@patrickhlauke
Copy link
Member

@alastc
Copy link
Contributor Author

alastc commented Jan 16, 2018

In which case we're flip-flopping on this, as we had scoped it to motion & scaling animations, and moved back general animation (prior to the comments period).

As a counter-point, is blurring a movement? Also, imaging scrolling down a page quickly and the colour flips back and forth at over 3Hz...

If this was at AA I'd agree it should be more specific, but the group thought that making general (at AAA) was the best way to go.

@patrickhlauke
Copy link
Member

As a counter-point, is blurring a movement? Also, imaging scrolling down a page quickly and the colour flips back and forth at over 3Hz...

then we're probably back at the problem of wanting to say "significant" animation or similar, which is impossible to normatively quantify/define.

@mbgower
Copy link
Contributor

mbgower commented Jan 16, 2018

then we're probably back at the problem of wanting to say "significant" animation or similar, which is impossible to normatively quantify/define.

We quantified this previously, however that was removed because we had insufficient data on what combination of length and size was a predictable trigger. So I wouldn't say it is necessarily impossible to quantify, but rather we lack data to define.

@alastc
Copy link
Contributor Author

alastc commented Jan 16, 2018

Proposal sent to the list:

SC: “Motion animation triggered by user interaction can be disabled, unless the animation is essential to the functionality or the information being conveyed.”

Def: “Motion animation: addition of steps between states to create the illusion of movement and/or to give a sense of movement.
For example, an element which moves into place or changes size while appearing is considered to be animated. An element which appears instantly in one frame is not using animation. Motion animation does not include changes of color, blurring or opacity.”

The above change covers the original intended scenario (vestibular disorders), if we also want to cover colour and other types of animation (e.g. for some cognitive folk) the definition should be changed to say something like:
“addition of steps between states to create the illusion of movement and/or to draw attention.” With some additional examples.

@alastc
Copy link
Contributor Author

alastc commented Jan 16, 2018

Closing as I've created a cleaner pull request.

@alastc alastc closed this Jan 16, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants