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
modernize addon: moved from webuniversum to appuniversum, converted to glimmer components #4
Merged
Merged
Changes from 9 commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
aa1866e
v3.14.0...v3.27.0
nvdk 603fae8
modernize components (glimmer + appuniversum)
nvdk 3cd2545
fix linting errors
nvdk 305c2f8
Create Login and Switch provider components
Windvis cf36c4c
Convert Torii authenticator to native class
erikap ffef022
Refactor session authentication promise with async/await
erikap eef6855
Fix capitals in component classnames
erikap b69de79
Replace merge polyfill with Object.assign
erikap 9ad4085
Refactor acmidm-oauth2 torii-provider to native class
erikap 7c51dc9
Merge branch 'chore/modernize-code' into feat/add-provider-components
erikap 9dd366b
Rollback acmidm-oauth2 torii-provider
erikap 48e3ab9
Merge branch 'chore/modernize-code' into feat/add-provider-components
erikap 347af45
Merge pull request #5 from lblod/feat/add-provider-components
nvdk File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -4,7 +4,6 @@ | |
|
||
root = true | ||
|
||
|
||
[*] | ||
end_of_line = lf | ||
charset = utf-8 | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,6 +13,8 @@ | |
# misc | ||
/coverage/ | ||
!.* | ||
.*/ | ||
.eslintcache | ||
|
||
# ember-try | ||
/.node_modules.ember-try/ | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,56 +1,59 @@ | ||
'use strict'; | ||
|
||
module.exports = { | ||
root: true, | ||
parser: 'babel-eslint', | ||
parserOptions: { | ||
ecmaVersion: 2018, | ||
sourceType: 'module', | ||
ecmaFeatures: { | ||
legacyDecorators: true | ||
} | ||
legacyDecorators: true, | ||
}, | ||
}, | ||
plugins: [ | ||
'ember' | ||
], | ||
plugins: ['ember'], | ||
extends: [ | ||
'eslint:recommended', | ||
'plugin:ember/recommended' | ||
'plugin:ember/recommended', | ||
'plugin:prettier/recommended', | ||
], | ||
env: { | ||
browser: true | ||
}, | ||
rules: { | ||
'ember/no-jquery': 'error' | ||
browser: true, | ||
}, | ||
rules: {}, | ||
overrides: [ | ||
// node files | ||
{ | ||
files: [ | ||
'.eslintrc.js', | ||
'.prettierrc.js', | ||
'.template-lintrc.js', | ||
'ember-cli-build.js', | ||
'index.js', | ||
'testem.js', | ||
'blueprints/*/index.js', | ||
'config/**/*.js', | ||
'tests/dummy/config/**/*.js' | ||
'tests/dummy/config/**/*.js', | ||
], | ||
excludedFiles: [ | ||
'addon/**', | ||
'addon-test-support/**', | ||
'app/**', | ||
'tests/dummy/app/**' | ||
'tests/dummy/app/**', | ||
], | ||
parserOptions: { | ||
sourceType: 'script' | ||
sourceType: 'script', | ||
}, | ||
env: { | ||
browser: false, | ||
node: true | ||
node: true, | ||
}, | ||
plugins: ['node'], | ||
rules: Object.assign({}, require('eslint-plugin-node').configs.recommended.rules, { | ||
// add your custom rules and overrides for node files here | ||
}) | ||
} | ||
] | ||
extends: ['plugin:node/recommended'], | ||
}, | ||
{ | ||
// Test files: | ||
files: ['tests/**/*-test.{js,ts}'], | ||
extends: ['plugin:qunit/recommended'], | ||
}, | ||
], | ||
}; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,6 +12,7 @@ | |
/.env* | ||
/.pnp* | ||
/.sass-cache | ||
/.eslintcache | ||
/connect.lock | ||
/coverage/ | ||
/libpeerconnection.log | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
# unconventional js | ||
/blueprints/*/files/ | ||
/vendor/ | ||
|
||
# compiled output | ||
/dist/ | ||
/tmp/ | ||
|
||
# dependencies | ||
/bower_components/ | ||
/node_modules/ | ||
|
||
# misc | ||
/coverage/ | ||
!.* | ||
.eslintcache | ||
|
||
# ember-try | ||
/.node_modules.ember-try/ | ||
/bower.json.ember-try | ||
/package.json.ember-try |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
'use strict'; | ||
|
||
module.exports = { | ||
singleQuote: true, | ||
}; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
'use strict'; | ||
|
||
module.exports = { | ||
extends: 'recommended' | ||
extends: 'recommended', | ||
}; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
<AuButton @loading={{this.isAuthenticating}} @disabled={{this.isAuthenticating}} {{on "click" this.login}}> | ||
Meld u aan | ||
</AuButton> | ||
{{#if this.errorMessage}} | ||
<AuAlert @alertIcon="alert-triangle" @alertTitle={{this.errorMessage}} /> | ||
{{/if}} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,26 +1,31 @@ | ||
import Component from '@glimmer/component'; | ||
import { warn } from '@ember/debug'; | ||
import Component from '@ember/component'; | ||
import layout from '../templates/components/acmidm-login'; | ||
import { inject as service } from '@ember/service'; | ||
import { tracked } from '@glimmer/tracking'; | ||
import { action } from '@ember/object'; | ||
|
||
export default Component.extend({ | ||
layout, | ||
session: service('session'), | ||
actions: { | ||
login() { | ||
this.set('errorMessage', ''); | ||
this.set('isAuthenticating', true); | ||
this.session.authenticate('authenticator:torii', 'acmidm-oauth2').catch((reason) => { | ||
warn(reason.error || reason, { id: 'authentication.failure' }); | ||
export default class AcmidmLoginComponent extends Component { | ||
@tracked errorMessage = ''; | ||
@tracked isAuthenticating = false; | ||
@service session; | ||
|
||
if (reason.status == 403) | ||
this.set('errorMessage', 'U heeft geen toegang tot deze applicatie.'); | ||
else | ||
this.set('errorMessage', 'Fout bij het aanmelden. Gelieve opnieuw te proberen.'); | ||
}) | ||
.finally(() => { | ||
this.set('isAuthenticating', false); | ||
}); | ||
@action | ||
async login() { | ||
this.errorMessage = ''; | ||
this.isAuthenticating = true; | ||
|
||
try { | ||
await this.session.authenticate('authenticator:torii', 'acmidm-oauth2'); | ||
} catch (reason) { | ||
warn(reason.error || reason, { id: 'authentication.failure' }); | ||
|
||
if (reason.status == 403) | ||
this.errorMessage = 'U heeft geen toegang tot deze applicatie.'; | ||
else | ||
this.errorMessage = | ||
'Fout bij het aanmelden. Gelieve opnieuw te proberen.'; | ||
} finally { | ||
this.isAuthenticating = false; | ||
} | ||
} | ||
}); | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
<button ...attributes type="button" {{on "click" this.logout}}> | ||
{{#if (has-block)}} | ||
{{yield}} | ||
{{else}} | ||
Meld u af | ||
{{/if}} | ||
</button> |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of using (and depending on) Appuniversum, it might be nice to keep these components styling agnostic?
I think this can easily be achieved by yielding state instead of outputting html. That way projects can decide which components to use.
Depending on appuniversum will also create an extra maintenance burden since it will need to be bumped from time to time (especially since it's a 0.x release).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Most projects also seem to implement a "compact" version (which duplicates the logic) of this component for the main header. That would then no longer be needed either.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that's a great idea, but would keep this for backwards compatibility. perhaps adding a deprecation warning in the constructor?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the data-table we took the approach of a styling agnostic addon and another addon that extends it with Appuniversum, but that results in a heavier maintentance burden I must admit (and still makes it a breaking change for this addon atm).
I'm pro the deprecation warning in this PR. It paves the way to a styling agnostic version and thus a breaking release in the future. But I would implement that in a separate PR and not lump it on this one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I only see now that there is a PR #5 already 🙈
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I created a follow-up PR as a quick POC of what a styling agnostic version could look like. I don't think the deprecation warnings should be added here since there are no alternatives yet, but I can add them to that PR as well.