fixed code from review

src/css/buttons.styl

@@ -39,7 +39,10 @@
  display: block;
 
 /* CROP BUTTON */
-#tie-crop-button
+
+
+#tie-crop-button,
+#tie-crop-preset-button
  .{prefix}-button.apply
  margin-right: 24px;
  .{prefix}-button.preset.active svg > use.active

src/css/submenu.styl

@@ -24,6 +24,8 @@
  font-weight: normal;
  font-size: 11px;
  margin: 0 9px 0 9px;
+ .{prefix}-button.preset
+ margin: 0 9px 20px 5px;
  label
  display: inline-block;
  cursor: pointer;

src/js/action.js

@@ -321,27 +321,31 @@ export default {
  this.stopDrawingMode();
  this.ui.changeMenu('crop');
  },
- 'preset-none': () => {
- this.setCropzoneRect();
- this.ui.crop.changeApplyButtonStatus(false);
- },
- 'preset-square': () => {
- this.setCropzoneRect(1 / 1);
- },
- 'preset-3-2': () => {
- this.setCropzoneRect(3 / 2);
- },
- 'preset-4-3': () => {
- this.setCropzoneRect(4 / 3);
- },
- 'preset-5-4': () => {
- this.setCropzoneRect(5 / 4);
- },
- 'preset-7-5': () => {
- this.setCropzoneRect(7 / 5);
- },
- 'preset-16-9': () => {
- this.setCropzoneRect(16 / 9);
+ preset: presetType => {
+ switch (presetType) {
+ case 'preset-square':
+ this.setCropzoneRect(1 / 1);
+ break;
+ case 'preset-3-2':
+ this.setCropzoneRect(3 / 2);
+ break;
+ case 'preset-4-3':
+ this.setCropzoneRect(4 / 3);
+ break;
+ case 'preset-5-4':
+ this.setCropzoneRect(5 / 4);
+ break;
+ case 'preset-7-5':
+ this.setCropzoneRect(7 / 5);
+ break;
+ case 'preset-16-9':
+ this.setCropzoneRect(16 / 9);
+ break;
+ default:
+ this.setCropzoneRect();
+ this.ui.crop.changeApplyButtonStatus(false);
+ break;
+ }
  }
  }, this._commonAction());
  },

src/js/component/cropper.js

@@ -67,7 +67,6 @@ class Cropper extends Component {
  * Start cropping
  */
  start() {
- console.log('START - CROPPER');
  if (this._cropzone) {
  return;
  }
@@ -278,55 +277,61 @@ class Cropper extends Component {
  };
  }
 
- /*
+ /**
  * Set a cropzone square
+ * @param {number} factor - preset ratio
+ * @returns {{left: number, top: number, width: number, height: number}}
  */
- setCropzoneRect(factor) {
-
- console.log('FACTOR - ', factor);
-
+ _getPresetCropSizePosition(factor) {
  const canvas = this.getCanvas();
- canvas.deactivateAll();
- canvas.selection = false;
-
- const cropzone = this._cropzone;
- cropzone.remove();
-
- if (!factor) {
- return;
- }
-
  const originalWidth = canvas.getWidth();
  const originalHeight = canvas.getHeight();
+
  const standardSize = (originalWidth >= originalHeight) ? originalWidth : originalHeight;
+ const getScale = (value, orignalValue) => (value > orignalValue) ? orignalValue / value : 1;
 
  let width = (standardSize * factor);
  let height = standardSize;
- let scale = 1;
 
- if (width > originalWidth) {
- scale = (originalWidth / width);
- width = (width * scale);
- height = (height * scale);
- }
+ const scaleWidth = getScale(width, originalWidth);
+ [width, height] = [width, height].map(sizeValue => (sizeValue * scaleWidth));
 
- if (height > originalHeight) {
- scale = (originalHeight / height);
- width = (width * scale);
- height = (height * scale);
- }
+ const scaleHeight = getScale(height, originalHeight);
+ [width, height] = [width, height].map(sizeValue => (sizeValue * scaleHeight));
+
+ return {
+ top: (originalHeight - height) / 2,
+ left: (originalWidth - width) / 2,
+ width,
+ height
+ };
+ }
+
+ /**
+ * Set a cropzone square
+ * @param {number} factor - preset ratio
+ */
+ setCropzoneRect(factor) {
+ const canvas = this.getCanvas();
+ const cropzone = this._cropzone;
 
- cropzone.set({
- top: 0,
- left: 0,
- height,
- width
+ canvas.deactivateAll();
+ canvas.selection = false;
+ cropzone.remove();
+
+ cropzone.set(factor ? this._getPresetCropSizePosition(factor) : {
+ top: -10,
+ left: -10,
+ height: 1,
+ width: 1
  });
 
  canvas.add(cropzone);
- canvas.centerObject(cropzone);
- canvas.setActiveObject(cropzone);
  canvas.selection = true;
+
+ if (factor) {
+ canvas.setActiveObject(cropzone);
+ }
  }
 
  /**

src/js/ui/crop.js

@@ -16,24 +16,22 @@ class Crop extends Submenu {
  });
 
  this.status = 'active';
+
  this._els = {
  apply: this.selector('#tie-crop-button .apply'),
  cancel: this.selector('#tie-crop-button .cancel'),
- 'preset-none': this.selector('#tie-crop-button .preset-none'),
- 'preset-square': this.selector('#tie-crop-button .preset-square'),
- 'preset-3-2': this.selector('#tie-crop-button .preset-3-2'),
- 'preset-4-3': this.selector('#tie-crop-button .preset-4-3'),
- 'preset-5-4': this.selector('#tie-crop-button .preset-5-4'),
- 'preset-7-5': this.selector('#tie-crop-button .preset-7-5'),
- 'preset-16-9': this.selector('#tie-crop-button .preset-16-9')
+ preset: this.selector('#tie-crop-preset-button')
  };
+
+ this.defaultPresetButton = this._els.preset.querySelector('.preset-none');
  }
 
  /**
  * Add event for crop
  * @param {Object} actions - actions for crop
  * @param {Function} actions.crop - crop action
  * @param {Function} actions.cancel - cancel action
+ * @param {Function} actions.preset - cancel action
  */
  addEvent(actions) {
  this.actions = actions;
@@ -47,39 +45,14 @@ class Crop extends Submenu {
  this._els.apply.classList.remove('active');
  });
 
- this._els['preset-none'].addEventListener('click', () => {
- this.setPresetButtonActive('preset-none');
- this.actions['preset-none']();
- });
-
- this._els['preset-square'].addEventListener('click', () => {
- this.setPresetButtonActive('preset-square');
- this.actions['preset-square']();
- });
-
- this._els['preset-3-2'].addEventListener('click', () => {
- this.setPresetButtonActive('preset-3-2');
- this.actions['preset-3-2']();
- });
-
- this._els['preset-4-3'].addEventListener('click', () => {
- this.setPresetButtonActive('preset-4-3');
- this.actions['preset-4-3']();
- });
-
- this._els['preset-5-4'].addEventListener('click', () => {
- this.setPresetButtonActive('preset-5-4');
- this.actions['preset-5-4']();
- });
-
- this._els['preset-7-5'].addEventListener('click', () => {
- this.setPresetButtonActive('preset-7-5');
- this.actions['preset-7-5']();
- });
+ this._els.preset.addEventListener('click', event => {
+ const button = event.target.closest('.tui-image-editor-button.preset');
+ if (button) {
+ const [presetType] = button.className.match(/preset-[^\s]+/);
 
- this._els['preset-16-9'].addEventListener('click', () => {
- this.setPresetButtonActive('preset-16-9');
- this.actions['preset-16-9']();
+  this._setPresetButtonActive(button);
+  this.actions.preset(presetType);
+ }
  });
  }
 
@@ -95,7 +68,7 @@ class Crop extends Submenu {
  */
  changeStandbyMode() {
  this.actions.stopDrawingMode();
- this.setPresetButtonActive();
+ this._setPresetButtonActive();
  }
 
  /**
@@ -112,18 +85,17 @@ class Crop extends Submenu {
 
  /**
  * Set preset button to active status
- * @param {String} preset - preset name
+ * @param {HTMLElement} button - event target element
  */
- setPresetButtonActive(preset) {
- const excludeList = [preset, 'apply', 'cancel'];
- Object.keys(this._els).forEach(key => {
- if (!excludeList.includes(key)) {
- this._els[key].classList.remove('active');
- }
+ _setPresetButtonActive(button = this.defaultPresetButton) {
+ const presetButtons = Array.from(this._els.preset.querySelectorAll('.preset'));
+
+ presetButtons.forEach(presetButton => {
+ presetButton.classList.remove('active');
  });
 
- if (preset) {
- this._els[preset].classList.add('active');
+ if (button) {
+ button.classList.add('active');
  }
  }
 }

src/js/ui/template/style.js

@@ -33,6 +33,7 @@ export default ({
  #tie-flip-button.flipY .tui-image-editor-button.flipY label,
  #tie-flip-button.resetFlip .tui-image-editor-button.resetFlip label,
  #tie-crop-button .tui-image-editor-button.apply.active label,
+ #tie-crop-preset-button .tui-image-editor-button.preset.active label,
  #tie-shape-button.rect .tui-image-editor-button.rect label,
  #tie-shape-button.circle .tui-image-editor-button.circle label,
  #tie-shape-button.triangle .tui-image-editor-button.triangle label,

src/js/ui/template/submenu/crop.js

@@ -1,6 +1,6 @@
 export default ({iconStyle: {normal, active}}) => (`
  <ul class="tui-image-editor-submenu-item">
- <li id="tie-crop-button">
+ <li id="tie-crop-preset-button">
  <div class="tui-image-editor-button preset preset-none active">
  <div>
  <svg class="svg_ic-submenu">
@@ -81,6 +81,9 @@ export default ({iconStyle: {normal, active}}) => (`
  </li>
  <li class="tui-image-editor-partition tui-image-editor-newline">
  </li>
+ <li class="tui-image-editor-partition only-left-right">
+ <div></div>
+ </li>
  <li id="tie-crop-button" class="action">
  <div class="tui-image-editor-button apply">
  <svg class="svg_ic-menu">

test/cropper.spec.js

@@ -246,7 +246,7 @@ describe('Cropper', () => {
  spyOn(cropper._cropzone, 'isValid').and.returnValue(true);
  cropper.setCropzoneRect(1 / 1);
  expect(cropper.getCropzoneRect()).toBeTruthy();
- expect(cropper.getCropzoneRect().width).toEqual(cropper.getCropzoneRect().height);
+ expect(cropper.getCropzoneRect().width).toBe(cropper.getCropzoneRect().height);
  cropper.end();
  });
 
@@ -255,7 +255,8 @@ describe('Cropper', () => {
  spyOn(cropper._cropzone, 'isValid').and.returnValue(true);
  cropper.setCropzoneRect(3 / 2);
  expect(cropper.getCropzoneRect()).toBeTruthy();
- expect(cropper.getCropzoneRect().width).toEqual(cropper.getCropzoneRect().height * (3 / 2));
+ expect((cropper.getCropzoneRect().width / cropper.getCropzoneRect().height).toFixed(1))
+ .toBe((3 / 2).toFixed(1));
  cropper.end();
  });
 
@@ -264,7 +265,8 @@ describe('Cropper', () => {
  spyOn(cropper._cropzone, 'isValid').and.returnValue(true);
  cropper.setCropzoneRect(4 / 3);
  expect(cropper.getCropzoneRect()).toBeTruthy();
- expect(cropper.getCropzoneRect().width).toEqual(cropper.getCropzoneRect().height * (4 / 3));
+ expect((cropper.getCropzoneRect().width / cropper.getCropzoneRect().height).toFixed(1))
+ .toBe((4 / 3).toFixed(1));
  cropper.end();
  });
 
@@ -273,7 +275,8 @@ describe('Cropper', () => {
  spyOn(cropper._cropzone, 'isValid').and.returnValue(true);
  cropper.setCropzoneRect(5 / 4);
  expect(cropper.getCropzoneRect()).toBeTruthy();
- expect(cropper.getCropzoneRect().width).toEqual(cropper.getCropzoneRect().height * (5 / 4));
+ expect((cropper.getCropzoneRect().width / cropper.getCropzoneRect().height).toFixed(1))
+ .toBe((5 / 4).toFixed(1));
  cropper.end();
  });
 
@@ -282,7 +285,8 @@ describe('Cropper', () => {
  spyOn(cropper._cropzone, 'isValid').and.returnValue(true);
  cropper.setCropzoneRect(7 / 5);
  expect(cropper.getCropzoneRect()).toBeTruthy();
- expect(cropper.getCropzoneRect().width).toEqual(cropper.getCropzoneRect().height * (7 / 5));
+ expect((cropper.getCropzoneRect().width / cropper.getCropzoneRect().height).toFixed(1))
+ .toBe((7 / 5).toFixed(1));
  cropper.end();
  });
 
@@ -291,7 +295,8 @@ describe('Cropper', () => {
  spyOn(cropper._cropzone, 'isValid').and.returnValue(true);
  cropper.setCropzoneRect(16 / 9);
  expect(cropper.getCropzoneRect()).toBeTruthy();
- expect(cropper.getCropzoneRect().width).toEqual(cropper.getCropzoneRect().height * (16 / 9));
+ expect((cropper.getCropzoneRect().width / cropper.getCropzoneRect().height).toFixed(1))
+ .toBe((16 / 9).toFixed(1));
  cropper.end();
  });