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

Break up annotations and shapes components #833

Merged
merged 5 commits into from
Aug 10, 2016
Merged

Conversation

etpinard
Copy link
Contributor

@etpinard etpinard commented Aug 8, 2016

... into digestible files.

src/components/annotations/ :
  - index.js 
  - attributes.js
  - constants.js // which includes the arrow paths
  - annotation_defaults.js
  - defaults.js
  - calc_autorange.js 
  - draw.js // which includes two methods: `draw` and `drawOne`
  - draw_arrow_head.js 

src/components/shapes/ :
  - index.js 
  - helpers.js // data to pixel convertion methods which hopefully will be reused elsewhere
  - attributes.js
  - constants.js  // mostly path params
  - shape_defaults.js 
  - defaults.js
  - calc_autorange.js 
  - draw.js // which includes two methods: `draw` and `drawOne`

This break up will make it easier to share code between annotations, shapes and other layout components.

Moreover, in this PR

  • the old Annotations.drawAll and Shapes.drawAll were replaced by draw consistent with other layout components
  • Plotly.relayout calls the one-item draw method which used to be called draw, but now rename drawOne. This pattern should will be clean up in the future.
  • DRYs up the shape jasmine test suite (thanks to helpers.js)
  • Adds relayout tests for annotations (we didn't have any yet 😉 )

- rename Annotations.drawAll -> Annotations.draw
- make old Annotatios.draw a private method in annotations/draw.js
- rename Shapes.drawAll -> Shapes.draw
- make old Shapes.draw a private method in shapes/draw.js
- make old Shapes.convertPath a private method in shapes/draw.js
- rm Shapes.add (wasn't used anywhere)
@etpinard etpinard added this to the Decirculise the src directory dependencies milestone Aug 10, 2016
@etpinard etpinard merged commit c0c0ad6 into master Aug 10, 2016
@etpinard etpinard deleted the component-break-up branch August 10, 2016 20:49
coerce('borderpad');

var borderWidth = coerce('borderwidth');
var showArrow = coerce('showarrow');
Copy link
Contributor

Choose a reason for hiding this comment

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

🐄 var. One or the other 😛

@mdtusz
Copy link
Contributor

mdtusz commented Aug 11, 2016

Well, it's already merged, but 💃 anyways!

@etpinard etpinard mentioned this pull request Aug 23, 2016
14 tasks
etpinard added a commit that referenced this pull request Aug 24, 2016
- use 'drawOne' mehtod instead of 'draw' as per PR #833
- use Registry instead of Plotly[module] as per PR #845
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.

2 participants