Skip to content
This repository has been archived by the owner on Sep 6, 2021. It is now read-only.

Implement steps() timing function editor #5799

Merged
merged 12 commits into from
Jan 3, 2014
Merged

Conversation

redmunds
Copy link
Contributor

  1. I added support for steps() timing function.
  2. I refactored the existing inline timing function code for cubic-bezier().
  3. I developed this using TDD, so this includes Unit Tests.

Support for:

  • steps(<integer>[,[start|end]]) function
  • step-start
  • step-end

Keyboard Events

  • up/down arrows control steps
  • left/right arrows control start/end

Mouse support

  • None. Doesn't seem like any natural gestures.

Issues?

  • Edit controls?
    • count spin control
    • toggle switch for start/end
  • More then ~25 steps is not usable
    • dynamically make lines thinner as number of steps grows?
    • increase size of editor?
    • rescale grid?
  • Editing doc while editor open is unforgiving
    • e.g. While typing "start", editor closes due to intermediate updates
  • Validation
  • editor silently fails when invalid parameters
  • Proposed Showing Errors for Inline Editors to help solve this issue. Note this also helps cubic-bezier() and all other Inline Editors.

@redmunds
Copy link
Contributor Author

@larz0 Here is the steps() function code.

@ghost ghost assigned larz0 Nov 1, 2013
@JeffryBooher
Copy link
Contributor

Nominating for Sprint 35

@redmunds
Copy link
Contributor Author

@larz0 This pull request was nominated for Sprint 35. Can you take a look and respond with what you think needs to be done to this feature for it to get into master? Then we can figure out if we can get it done this sprint.

@larz0
Copy link
Member

larz0 commented Nov 21, 2013

@redmunds ok I'll take a look this afternoon.

@redmunds
Copy link
Contributor Author

redmunds commented Dec 4, 2013

@larz0 I added some info text for both the cubic-bezier and steps editors. Let me know what you think.

It's going to be a fair amount of work to display error messages (need separate test and string for every possible error), so I want to make sure this is on the right track before proceeding.

If it's OK, then feel free to tweak position, styles, text, etc.

@redmunds
Copy link
Contributor Author

redmunds commented Jan 2, 2014

@larz0 I thought you asked me to add symbols for arrow keys to the info text (but I can't seem to find your comment), so I added those. I looked for unicode symbols for mouse click and drag operations, but couldn't find anything -- I suppose we could use images.

Let me know what else you think needs to be done to merge this into master. In the spirit of agile, maybe we should merge what we have and listen for feedback?

@larz0
Copy link
Member

larz0 commented Jan 2, 2014

I'd like to make it more like the screenshot below. What do you think? I can push the changes to this PR.

screen shot 2014-01-02 at 1 06 33 pm

@redmunds
Copy link
Contributor Author

redmunds commented Jan 2, 2014

I like that! Is that format used anywhere else in Brackets so I could re-use the boxes and background-color?

I also added info text to cubic-bezier() editor, so that's why I was asking about mouse click and drag. Suggestions?

@larz0
Copy link
Member

larz0 commented Jan 2, 2014

Awesome. I'll make those changes in cubic-bezier as well. I'm going to remove "Bezier curve endpoints can be dragged with mouse." because it's pretty clear that they can be dragged as they look like typical bezier curve handles.

@larz0
Copy link
Member

larz0 commented Jan 2, 2014

@redmunds I've made the changes, might want to take a look just in case.

@redmunds
Copy link
Contributor Author

redmunds commented Jan 2, 2014

Looks awesome! Let me know when you're happy with XD side, and I'll get a dev to give this a code review.

@larz0
Copy link
Member

larz0 commented Jan 2, 2014

@redmunds I'm happy with XD side. Let's try and merge this :)

@redmunds
Copy link
Contributor Author

redmunds commented Jan 2, 2014

XD review is complete. Ready for Dev code review.

@@ -477,6 +477,8 @@ define({
// extensions/default/InlineTimingFunctionEditor
"INLINE_TIMING_EDITOR_TIME" : "Time",
"INLINE_TIMING_EDITOR_PROGRESSION" : "Progression",
"BEZIER_EDITOR_INFO" : "<kbd>↑</kbd><kbd>↓</kbd><kbd>←</kbd><kbd>→</kbd> Move selected point<br><kbd class='text'>Shift</kbd> Move by ten units",
"STEPS_EDITOR_INFO" : "<kbd>↑</kbd><kbd>↓</kbd> Increase or decrease steps.<br><kbd>←</kbd><kbd>→</kbd> 'Start' or 'End'",
Copy link
Contributor

Choose a reason for hiding this comment

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

After steps there is a period, but in the other 3 cases of "end of line" there isn't.
These should be standardized.

@ghost ghost assigned bchintx Jan 3, 2014
@bchintx
Copy link
Contributor

bchintx commented Jan 3, 2014

Done with code review. Looks and works great! merging.

bchintx added a commit that referenced this pull request Jan 3, 2014
Implement steps() timing function editor
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.

5 participants