Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fixing Error in Hds::Flyout & Hds::Modal subcomponents #2511

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
+20 −6
Open

Fixing Error in Hds::Flyout & Hds::Modal subcomponents #2511

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
00c8867
fixing ts errors
zamoore Oct 18, 2024
de3cd84
added changeset
zamoore Oct 18, 2024
e93a689
fixed super issue in hds modal body
zamoore Oct 21, 2024
0e55134
Update changeset
zamoore Oct 21, 2024
1635c55
correcting ts issue
zamoore Oct 21, 2024
f2146e5
correcting ts issue
zamoore Oct 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions .changeset/dull-lamps-retire.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
---
"@hashicorp/design-system-components": minor
---

`Hds::DialogPrimitive`
- Fixed error in `Description` and `Body` subcomponents, by not passing the `args` argument from the constructor to `super`
- Added missing arguments in `Arg` TypeScript signature object

`Hds::Modal`
- Fixed error in `Body` subcomponent, caused by not passing the `args` argument from the constructor to `super`
- Added missing arguments in `Arg` TypeScript signature object
Comment on lines +5 to +11
Copy link
Member

Choose a reason for hiding this comment

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

The changelog would need to be updated, as we're not making any changes to the DialogPrimitive and there are no missing arguments anymore.

5 changes: 3 additions & 2 deletions packages/components/src/components/hds/flyout/body.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,16 @@ import Component from '@glimmer/component';
import { deprecate } from '@ember/debug';

export interface HdsFlyoutBodySignature {
Args: never;
Copy link
Contributor

Choose a reason for hiding this comment

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

[question] as a pattern, do we prefer to have Args completely missing (I think I have done something like this, recently), or Args: never like here? which one is more "correct™"?

/cc @alex-ju

Copy link
Contributor

Choose a reason for hiding this comment

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

Or is it because below we need to have a HdsFlyoutBodySignature['Args']?

Copy link
Member

Choose a reason for hiding this comment

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

[question] as a pattern, do we prefer to have Args completely missing (I think I have done something like this, recently), or Args: never like here? which one is more "correct™"?

not having Args is the established pattern. however, considering the error we encounter in HCP for this subcomponent, I don't see a way of avoiding never.

we could use never straight in the constructor, but we have this pattern of using the signature in constructors, so what we have in this PR feels more consistent.

constructor(owner: unknown, args: never) {

Copy link
Contributor

Choose a reason for hiding this comment

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

not having Args is the established pattern

@alex-ju Good. So nothing to change on my side :)

however, considering the error we encounter in HCP for this subcomponent, I don't see a way of avoiding never.

@alex-ju @zamoore since this is a deviation from the pattern, should we add a comment above these Args: never are not following the established pattern? (and potentially have also a link to this thread, for future reference)

Blocks: {
default: [];
};
Element: HTMLDivElement;
}

export default class HdsFlyoutBody extends Component<HdsFlyoutBodySignature> {
constructor(owner: unknown) {
super(owner, {});
constructor(owner: unknown, args: HdsFlyoutBodySignature['Args']) {
super(owner, args);

deprecate(
'The `Hds::Flyout::Body` sub-component is now deprecated and will be removed in the next major version of `@hashicorp/design-system-components`. Use `Hds::DialogPrimitive::Body` as one-to-one replacement.',
Expand Down
5 changes: 3 additions & 2 deletions packages/components/src/components/hds/flyout/description.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,16 @@ import { deprecate } from '@ember/debug';
import type { HdsTextBodySignature } from '../text/body';

export interface HdsFlyoutDescriptionSignature {
Args: never;
Blocks: {
default: [];
};
Element: HdsTextBodySignature['Element'];
}

export default class HdsFlyoutDescription extends Component<HdsFlyoutDescriptionSignature> {
constructor(owner: unknown) {
super(owner, {});
constructor(owner: unknown, args: HdsFlyoutDescriptionSignature['Args']) {
super(owner, args);

deprecate(
'The `Hds::Flyout::Description` sub-component is now deprecated and will be removed in the next major version of `@hashicorp/design-system-components`. Use `Hds::DialogPrimitive::Description` as one-to-one replacement.',
Expand Down
5 changes: 3 additions & 2 deletions packages/components/src/components/hds/modal/body.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,16 @@ import Component from '@glimmer/component';
import { deprecate } from '@ember/debug';

export interface HdsModalBodySignature {
Args: never;
Blocks: {
default: [];
};
Element: HTMLDivElement;
}

export default class HdsModalBody extends Component<HdsModalBodySignature> {
constructor(owner: unknown) {
super(owner, {});
constructor(owner: unknown, args: HdsModalBodySignature['Args']) {
super(owner, args);

deprecate(
'The `Hds::Modal::Body` sub-component is now deprecated and will be removed in the next major version of `@hashicorp/design-system-components`. Use `Hds::DialogPrimitive::Body` as one-to-one replacement.',
Expand Down