-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
UI: Add Typescript for PKI engine #17927
Conversation
An old version of prettier-eslint-cli was requiring typescript@3.9.9, which triggered the DefinitelyTyped issue [#53397](DefinitelyTyped/DefinitelyTyped#53397)
f9f43bf
to
3644b01
Compare
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.
Really nice job!
@onChange={{this.checkboxChange}} | ||
data-test-key-usage-key-usage-checkboxes | ||
/> | ||
<CheckboxGrid |
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.
😍 so clean! 👏
label: string; | ||
} | ||
|
||
const KEY_USAGE_FIELDS: Field[] = [ |
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.
Not for now, but just as a spitballing note - wondering if these could live in a pki helper if we have any other fields that could/will be stored as const
s. Otherwise - do we need to define these here? Or can we directly set these values on keyUsageFields
and extKeyUsageFields
below?
import { hbs } from 'ember-cli-htmlbars'; | ||
import Sinon from 'sinon'; | ||
|
||
module('Integration | Component | checkbox-grid', function (hooks) { |
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.
👏 👏 👏
return []; | ||
}, | ||
}) | ||
extKeyUsage; |
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.
Nice! Did you add this in case this attr isn't a consistent type as well?
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.
Yeah, I needed to define this attribute as an array because without it, trying to set the value to an array was falling back to just the string. Probably some updates we can make in openApiToAttrs
to address this but figured that was out of scope for this
Add Typescript as a dependency, so we can use it in forthcoming work 🎉
To start, I created a new TS/Glimmer component in the
core
shared lib, and also typed an existing PKI component