Feature/ember upgrade by josex2r · Pull Request #153 · BBVAEngineering/ember-modal-service

josex2r · 2021-11-24T07:59:34Z

No description provided.

BRAKING CHANGES: upgrade Ember version

codecov · 2021-11-24T08:13:42Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (7791d42) to head (45448aa).
⚠️ Report is 16 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff            @@
##            master      #153   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            6         7    +1     
  Lines           82       100   +18     
=========================================
+ Hits            82       100   +18

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

# Conflicts: # README.md # addon/components/modal.js # addon/services/modal.js

BRAKING CHANGE: modal component extends from Glimmer

…the name

BREAKING CHANGE: promise is fulfilled when the modal animation ends

adrigzr · 2021-12-14T15:56:29Z


-	init() {
-		super.init(...arguments);
+  get componentName() {


This should be inside modal-wrapper and not in the model.

xe21500 · 2021-12-14T15:58:48Z

+    return () => {
+      this.modal
+        .open(modalName, options)
+        .then(actions.onDone, actions.onFail, 'open-modal helper');


async / await here?

xe21500 · 2021-12-15T06:58:45Z

+    this.trigger('open', model);
+
+    model.promise
+      .catch(() => {})


I'd try...catch here instead

SergeAstapov · 2021-12-21T18:49:41Z

@@ -0,0 +1,3 @@
+<div role="dialog" data-id={{this.dataId}} data-modal-show={{this.dataModalShow}} {{did-insert this.onDidInsert}}>
+  <@model.componentName @model={{@model}} @changeVisibility={{this.changeVisibility}} @element={{this.element}} />


@josex2r I may be wrong, but this should not work per emberjs/ember.js#17744

I assume, for embroider compatibility we should use ensure-safe-component helper provided by @embroider/util package like so

{{#let (ensure-safe-component (component @model.componentName)) as |Modal|}} <Modal @model={{@model}} @changeVisibility={{this.changeVisibility}} @element={{this.element}} /> {{/let}}

see docs https://github.com/embroider-build/embroider/tree/master/packages/util#the-utilities

my bad, didn't see it's actually being used in model.js.
IMO it's better to do component/template manipulations in the hbs file

Nice! I didn't read that the helper exists XD

SergeAstapov · 2021-12-21T18:52:32Z

+  @tracked isRejected = false;

-	@tracked _deferred;
+  @tracked _deferred = defer();


_deferred does not change during object lifecycle, maybe we don't need to use @tracked here?

SergeAstapov · 2021-12-21T18:57:43Z

+
+  @action
+  resolve(data) {
+    this._fullfillmentFn = () => this.model.resolve(data);


minor, this may be implemented using destroyable like so

registerDestructor(this, () => this.model.resolve(data));

not a big deal at all, but IMO it's easier to follow the code flow

SergeAstapov · 2021-12-21T19:05:04Z

+
+  @isNotDestroyed
+  _safeDidOpen() {
+    this.didOpen && this.didOpen();


should this be this.args.didOpen?

No, this function is something like a component life-cycle hook, to notify when the modal is visible.

Was created to extend the modal class and prevent checking if a CSS transition exists and attaching to it using more callbacks:

// components/modal-foo.js class ModalFoo extends ModalComponent { didOpen() { // Something to do here... } }

@josex2r got it! Thanks for explanation!

SergeAstapov · 2021-12-21T19:11:48Z

-/.nycrc.json
-/.releaserc
-/greenkeeper.json
-/jsconfig.json


jsconfig.json probably should be kept as it should not be published to npm

SergeAstapov · 2021-12-21T19:14:36Z

@josex2r Thanks for working on this!

# Conflicts: # addon/components/modal.js # addon/services/modal.js # package.json # tests/acceptance/modal-component-test.js # tests/dummy/app/components/modal-custom-modal/index.hbs # tests/dummy/app/styles/app.css # tests/integration/services/modal-test.js # yarn.lock

SergeAstapov · 2022-05-23T19:53:43Z

@josex2r @xe21500 @adrigzr is there something I can help to push this over the line and get released? I'm happy to help maintaining this addon.

If that could help, I can help and split changes proposed in this PR into smaller PRs that are easier to review - if that can help expedite things.

In addition to general Ember upgrades and transition to Glimmer component, this addon also great candidate for v2 format conversion

adrigzr · 2022-05-24T06:08:35Z

@etarancon, could you help @SergeAstapov with this? 😄

SergeAstapov · 2022-11-29T11:34:30Z

@josex2r @etarancon @adrigzr would you like to add me as maintainer of the addon as I can get this, general addon upgrades/maintenance and v2 format conversion done

next-joserepresa and others added 10 commits November 23, 2021 09:01

feat: wait until modal is destroyed

d5bec8f

docs: update README

e1ce8f5

test: fix coverage

d4d6687

feat: upgrade Ember version v3.21.2...v3.28.4

d575196

BRAKING CHANGES: upgrade Ember version

style: prettier

32f9fb8

style: test prettier rules

49a8d69

feat: bound resolve and reject to be called as actions

d9b5963

refactor(db-254623): more Octane upgrades

03e8fef

ci(db-254623): github actions and upgrade husky hooks

b813884

ci(db-254623): upgrade husky version

d1f5fae

josex2r added 6 commits November 25, 2021 17:09

feat: remove deprecations by upgrading babel

32a1e96

Merge remote-tracking branch 'origin/master' into feature/ember-upgrade

604a3cd

# Conflicts: # README.md # addon/components/modal.js # addon/services/modal.js

test: remove runloop functions

eda7cfa

feat: upgrade ember-auto-import to v2

c16d5fd

feat(octane): migrate to Glimmer components

f6d262a

BRAKING CHANGE: modal component extends from Glimmer

feat(octane): remove deprecation by using component class instead of …

c9be982

…the name

josex2r mentioned this pull request Dec 1, 2021

Glimmer components support #104

Open

feat: upgrade ember-task-scheduler

1673529

josex2r force-pushed the feature/ember-upgrade branch from 286ffa5 to 1673529 Compare December 1, 2021 10:56

josex2r added 5 commits December 1, 2021 12:03

feat: add test-waiters to deps to work with ember-embroider-safe

fa52cdb

feat: wait close animation to change promise state

7b0cd04

BREAKING CHANGE: promise is fulfilled when the modal animation ends

feat: move test-waiters to deps

1377735

test: do not wait for click

fe36e11

feat: remove ember-task-scheduler

9a64965

adrigzr reviewed Dec 14, 2021

View reviewed changes

xe21500 reviewed Dec 14, 2021

View reviewed changes

xe21500 reviewed Dec 15, 2021

View reviewed changes

xe21500 previously approved these changes Dec 15, 2021

View reviewed changes

SergeAstapov reviewed Dec 21, 2021

View reviewed changes

josex2r added 3 commits December 23, 2021 11:50

fix: update DOM when closing modal via service

31d319b

refactor: add some PR #153 suggestions

4ebec8f

josex2r dismissed xe21500’s stale review via 45448aa December 23, 2021 11:12

eumendoza approved these changes Nov 16, 2022

View reviewed changes

		@@ -0,0 +1,3 @@
		<div role="dialog" data-id={{this.dataId}} data-modal-show={{this.dataModalShow}} {{did-insert this.onDidInsert}}>
		<@model.componentName @model={{@model}} @changeVisibility={{this.changeVisibility}} @element={{this.element}} />

Uh oh!

Conversation

josex2r commented Nov 24, 2021

Uh oh!

codecov Bot commented Nov 24, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SergeAstapov Dec 21, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SergeAstapov commented Dec 21, 2021

Uh oh!

SergeAstapov commented May 23, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

adrigzr commented May 24, 2022

Uh oh!

SergeAstapov commented Nov 29, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

codecov Bot commented Nov 24, 2021 •

edited

Loading

SergeAstapov Dec 21, 2021 •

edited

Loading

SergeAstapov commented May 23, 2022 •

edited

Loading