Skip to content
This repository was archived by the owner on Aug 2, 2024. It is now read-only.

Refactor polaris-drop-zone#359

Merged
vladucu merged 11 commits into
refactor/es6-classesfrom
refactor/es6-polaris-drop-zone
Aug 23, 2019
Merged

Refactor polaris-drop-zone#359
vladucu merged 11 commits into
refactor/es6-classesfrom
refactor/es6-polaris-drop-zone

Conversation

@vladucu
Copy link
Copy Markdown
Member

@vladucu vladucu commented Aug 7, 2019

What's here

  • refactors to ES6 class
  • template-only components
  • angle-bracket syntax
NOTE

Tests are failing on beta & canary, likely related to ember-decorators/ember-decorators#451

@vladucu vladucu self-assigned this Aug 7, 2019
@vladucu vladucu force-pushed the refactor/es6-polaris-drop-zone branch from 0c22277 to e333457 Compare August 8, 2019 10:32
@vladucu vladucu changed the base branch from refactor/es6-polaris-button to refactor/es6-classes August 8, 2019 11:51
@vladucu vladucu force-pushed the refactor/es6-polaris-drop-zone branch from e333457 to 0c98746 Compare August 8, 2019 11:55
@vladucu vladucu force-pushed the refactor/es6-polaris-drop-zone branch 2 times, most recently from f0772ac to 43ea1c8 Compare August 9, 2019 12:47
Copy link
Copy Markdown
Contributor

@tomnez tomnez left a comment

Choose a reason for hiding this comment

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

This is awesome 🙏 Just a couple comments, nothing major.

'state',
'sizeClass'
);
@computed()
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

do we want to keep the .readOnly() on all these CPs?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

no, having a computed property with only a getter....trying to set it, should throw an error

* input element works. This should be a user activation.
*
* Following warning will be logged
* `File chooser dialog can only be shown with a user activation.`
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

👍

(component "wrapper-element" tagName="")
) as |Wrapper|}}
<Wrapper
@id={{this.id}}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should this be @id instead of this.id since id is a public attribute?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

yes indeed...good catch
fixed

@vladucu vladucu force-pushed the refactor/es6-polaris-drop-zone branch from 43ea1c8 to 940d938 Compare August 12, 2019 16:47
@vladucu vladucu requested a review from tomnez August 12, 2019 16:47
Copy link
Copy Markdown
Contributor

@tomnez tomnez left a comment

Choose a reason for hiding this comment

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

LGTM 🥇

@vladucu vladucu force-pushed the refactor/es6-classes branch from ca07506 to 04f5786 Compare August 23, 2019 22:22
@vladucu vladucu force-pushed the refactor/es6-polaris-drop-zone branch from 940d938 to 43d372b Compare August 23, 2019 22:29
@vladucu vladucu merged commit 43d372b into refactor/es6-classes Aug 23, 2019
@vladucu vladucu deleted the refactor/es6-polaris-drop-zone branch August 23, 2019 22:55
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants