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.

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

feat(type-expand-provider.ts): expand the first level of tree item automatically #379

Merged
merged 2 commits into from
Sep 14, 2023

Conversation

senkenn
Copy link
Contributor

@senkenn senkenn commented Sep 8, 2023

@senkenn senkenn changed the title feat(type-expand-provider.ts): expand the tree item only first level feat(type-expand-provider.ts): expand the first level of tree item automatically Sep 8, 2023
@senkenn senkenn mentioned this pull request Sep 8, 2023
Copy link
Owner

@d-kimuson d-kimuson left a comment

Choose a reason for hiding this comment

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

@senkenn PRありがとうございます!質問とコメントを書いたので確認お願いします!

@@ -77,6 +90,7 @@ export class TypeExpandProvider
}

this.selection = selection
this.topElement = undefined
Copy link
Owner

Choose a reason for hiding this comment

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

📝

  • 選択している Node が変わったときに topElement をリセット
  • topElement (リセット後初めて getElement で呼ばれたもの) なら展開して topElement 持たせる → 2回目以降は topElement に Elemetn があるので更新されているので折りたたんだ状態に更新して返す

@@ -16,6 +16,7 @@ export class TypeExpandProvider
type: TypeObject
}
private activeFilePath: string | undefined
private topElement?: ExpandableTypeItem
Copy link
Owner

Choose a reason for hiding this comment

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

[want] 欲しいのは表示済みかの状態だけなので、boolean で十分で意図が読みやすいかなと思いました!Element 自体持たせている意図が特になければこちらでお願いしたいです!

Suggested change
private topElement?: ExpandableTypeItem
private isAlreadyShowTopElement: boolean = false

element.collapsibleState = vscode.TreeItemCollapsibleState.Expanded
this.topElement = element
} else {
element.collapsibleState = vscode.TreeItemCollapsibleState.Collapsed
Copy link
Owner

Choose a reason for hiding this comment

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

[q] ここの collapsibleState の更新って展開できないものは None、展開できるものは Collapsed で来ると思うので明示的な更新は不要そうに見えたんですが、わかりやすく明示している感じですかね?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

確かに不要ですね。手前の

    // The bottom layer element is not expandable
    if (element.collapsibleState === vscode.TreeItemCollapsibleState.None) {
      return element
    }

も結局「展開できないものは Noneで来る」を前提に条件書いてるのでelseは削除します。

d-kimuson added a commit that referenced this pull request Sep 11, 2023
@d-kimuson
Copy link
Owner

この PR に関する Changelog は記入済み
e0ae078

Approve 次第取り込んだバージョンをリリースします 🙆‍♂️

Copy link
Owner

@d-kimuson d-kimuson left a comment

Choose a reason for hiding this comment

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

LGTM

@d-kimuson d-kimuson merged commit 198c841 into d-kimuson:master Sep 14, 2023
1 check passed
@d-kimuson
Copy link
Owner

@senkenn PRありがとうございました!
先程この変更を取り込んだ v1.0.6 をリリースしました!
https://github.com/d-kimuson/ts-type-expand/releases/tag/v1.0.6

@senkenn senkenn deleted the feature/issue#337 branch September 14, 2023 11:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants