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

Improve code readability in examples #16

Closed
Martin-Pitt opened this issue Jul 30, 2018 · 3 comments
Closed

Improve code readability in examples #16

Martin-Pitt opened this issue Jul 30, 2018 · 3 comments
Assignees
Milestone

Comments

@Martin-Pitt
Copy link

There's two issues in the examples that stick out notably:

  • Code pane is small
  • Code itself is written in a compact style

Taking for example the following code:

const element = document.querySelector('#move');
const state = document.querySelector('[data-state]');

window.onload = () => setTimeout(() => { // Wait for page loading
  new Between({x: 0, y: 0, r: 0}, {x: 400, y: 30, r: 90})
    .time(2000)
    .easing(Between.Easing.Cubic.In)
    .on('update', v => {
      element.style.transform = `translate(${v.x}px, ${v.y}px) rotate(${v.r}deg)`;
    }).on('start', () => {
      state.innerText = 'state: running';
    }).on('complete', () => {
      state.innerText = 'state: complete';
    });
}, 2000);

An improvement could be:

// You can pass in any object with numbers to tween
const from = { x: 0, y: 0, rotation: 0 };
const to = { x: 400, y: 30, rotation: 90 };

function createAnimation() {
  new Between(from, to)
    .time(2000)
    .easing(Between.Easing.Cubic.In)
    .on('update', updateElementTransform)
    .on('start', onStarted)
    .on('complete', onComplete);
}

function updateElementTransform(value) {
  // `value` is same object as from/to but with new values
  const { x, y, rotation } = value;
  element.style.transform = `translate(${x}px, ${y}px) rotate(${rotation}deg)`;
}

function onStarted() {
  state.innerText = 'state: running';
}

function onComplete() {
  state.innerText = 'state: complete';
}

// Get the elements for our visual example and the state indicator
const element = document.querySelector('#move');
const state = document.querySelector('[data-state]');

// Create the animation in 2 seconds, after the page is fully loaded
window.onload = () => setTimeout(createAnimation, 2000);

Goal is to offload details at the end where possible.

It should introduce how to work with the API as succinctly as possible.

Using full function syntax allows them to be named and be recognisable. Arrow functions can be really great but experts are often tempted into using them in documentation, which can be hard on beginners to the library.

I introduce these new object values early on (as it is the object example) and the API for Between. It should be easier to see at an overview now.

I use destructuring because it is actually kinda really handy way to set expectations in code and add transparency to contents of an object being passed around.


Using same approach as above, could be used across the examples perhaps?

@sasha240100
Copy link
Owner

sasha240100 commented Jul 30, 2018

@Martin-Pitt So, based on the issue I would highlight the following genral rules that need to be required for examples:

(UPDATED)

  • Move from & to into variables for examples that use non-numbers
  • Shorten the width of code, move comments on the new (next/previous) lines
  • Move window.onload + setTimeout on the separate line
  • Add more comments explaining the code & API.

I would also argue that I see no need in separating onStarted, onComplete as the pattern of events is very common and used pretty everywhere plus ES6 is a 2 y.o. standard already, but thanks for the suggestion!

/ping @alexko30 @IlyaSenko

sasha240100 added a commit that referenced this issue Jul 30, 2018
@sasha240100
Copy link
Owner

@Martin-Pitt Can I close it?

@Martin-Pitt
Copy link
Author

Feel free to close it

@sasha240100 sasha240100 added this to the v0.1.2 milestone Aug 15, 2018
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

No branches or pull requests

4 participants