[view] 브랜치 변경 시 테마 초기화되는 문제 해결 및 코드 리팩토링 #767
Conversation
| @@ -53,7 +54,7 @@ const App = () => { | |||
| if (loading) { | |||
| return ( | |||
| <BounceLoader | |||
| color="#ff8272" | |||
| color={THEME_INFO[window.theme as keyof typeof THEME_INFO].colors.primary} | |||
There was a problem hiding this comment.
로딩 컴포넌트에도 선택한 theme 컬러가 적용되니 좋네요 👍👍
| @@ -54,10 +54,10 @@ export default class FakeIDEAdapter implements IDEPort { | |||
| this.sendMessageToMe(message); | |||
| } | |||
|
|
|||
| public setCustomTheme(color: string) { | |||
| sessionStorage.setItem("PRIMARY_COLOR", color); | |||
| public sendUpdateThemeMessage(theme: string) { | |||
| sendUpdateThemeCommand(value); | ||
| window.theme = value; | ||
| document.documentElement.setAttribute("theme", value); |
There was a problem hiding this comment.
Discussion에 써주신대로 window.theme을 상태로 대신할 수도 있겠네요 😲
| import type { ThemeInfo } from "./ThemeSelector.type"; | ||
|
|
||
| export const THEME_INFO: ThemeInfo = { |
| @@ -0,0 +1,11 @@ | |||
| export type ThemeInfo = { | |||
| [key in "githru" | "hacker-blue" | "aqua" | "cotton-candy" | "mono"]: { | |||
There was a problem hiding this comment.
무결성을 위해 key의 literal type을 강제한 것 매우 좋아보입니다!
추가로 theme key list를 별도로 빼도 괜찮을 것 같습니다!
There was a problem hiding this comment.
넵 그럼 다음 이슈에서 상수로 관리하도록 빼두겠습니다 ! ! !
| @@ -49,9 +49,9 @@ export default class VSCodeIDEAdapter implements IDEPort { | |||
| this.sendMessageToIDE(message); | |||
| } | |||
|
|
|||
| public setCustomTheme(theme: string) { | |||
| public sendUpdateThemeMessage(theme: string) { | |||
There was a problem hiding this comment.
함수명 적절하게 잘 바꿔주신 것 같아요!!!🥰🥰
| @@ -111,12 +111,12 @@ export default class WebviewLoader implements vscode.Disposable { | |||
| }); | |||
| } | |||
|
|
|||
| private async getWebviewContent(webview: vscode.Webview): Promise<string> { | |||
There was a problem hiding this comment.
(궁금) return type은 왜 삭제하게 되셨는지 궁금합니다.!!
There was a problem hiding this comment.
기존 함수를 async로 변경하고, Promise Type을 씌웠는데, 다시 동기 함수로 변경하는 과정에서 삭제가 되어버렸습니다 🥲
의도한 것은 아니고 기존 코드를 확인 못하고 제거한 부분인데, 일단 해당 함수가 반환하는 변수명이 returnString으로 string 타입을 반환한다는 부분이 명시적이고, 타입 추론으로 인해 코드상 에러는 없으니, 다음 이슈에 반환 타입을 기재하도록 하겠습니다 ㅎㅎ
Related issue
close #747 #533
Result
2024-10-13.3.16.15.mov
Work list
1. 브랜치 변경 시 테마 초기화되는 문제 해결
ThemeIcons컴포넌트에서 따로selectedItem변수를 사용하지 않고window.theme값을 사용할 수 있게 되어,selectedItem을window.theme으로 대체 후 제거했습니다.2. 테마 관련 상수와 타입을 분리
ThemeSelector.const.ts와ThemeSelector.type.ts를 생성해 따로 관리할 수 있도록 했습니다.3. 로딩 컴포넌트의 테마 색상값 적용
4. 기타 코드 리팩토링
Discussion
window.theme 값을 너무 자주 변경하게 되면 혹시나 발생하게 될 부수효과가 걱정됩니다. ....... 🥲
계속해서 직접 접근하는 대신, zustand를 사용해 처음 window.theme 값을 가져오고, 이후 zustand로 관리해주는 방법을 고민하고 있습니다 !!!