[view] Theme Selector 테마 전역 저장 및 코드 리팩토링 #760

taboowiths · 2024-10-06T11:32:54Z

Related issue

#752

Result

2024-10-06.8.11.20.mov

Work list

1. Theme Selector 테마 전역 저장

ThemeSelector를 이용해 선택한 테마를 재접속 시 유지할 수 있도록 구현했습니다.
로직은 다음과 같습니다:

ThemeSelector 컴포넌트를 통해 테마를 선택하여 변경합니다.
vscode api를 통해, vscode에 메시지를 보냅니다. (view/services)

 public setCustomTheme(theme: string) {
    const message: IDEMessage = {
      command: "updateCustomTheme",
      payload: JSON.stringify({ theme }),
    };
    this.sendMessageToIDE(message);
  }

vscode가 메시지를 수신, command에 맞는 동작을 수행합니다.
여기서는 setTheme() 함수를 호출하여, vscode의 configuration 중 githru.theme 값을 업데이트 합니다.

 // vscode/webview-loader.ts
 if (command === "updateCustomTheme") {
   const colorCode = payload && JSON.parse(payload);
   if (colorCode.theme) {
     setTheme(colorCode.theme);
   }
 }

// vscode/setting-repository.ts
export const setTheme = async (newTheme: string) => {
  const configuration = vscode.workspace.getConfiguration();
  configuration.update(SETTING_PROPERTY_NAMES.THEME, newTheme);
};

저장된 값은 다음 webview를 로드할 때 getTheme()함수를 통해 vscode configuration에 업데이트 한 theme 값을 가져와 window 객체에 저장합니다. (vscode/webview-loader.ts)
ThemeSelector 컴포넌트에서 window.theme 값을 가져와 테마를 업데이트 합니다.

2. 코드 리팩토링

#687 의 피드백을 반영하여 코드를 리팩토링 했습니다.
type 설정, 변수명 변경, 그리고 selector 영역 외 클릭 시 창이 닫히도록 구현했습니다.

Discussion

joonwonBaek

정현님 수고하셨습니다...!!
웹뷰쪽 코드가 까다로운데 잘 구현해주셨네요 👏👏👏

joonwonBaek · 2024-10-06T12:22:13Z

packages/view/src/components/ThemeSelector/ThemeSelector.tsx

+        {Object.values(colors).map((color, index) => (
+          <div
+            key={Number(index)}
+            className="theme-icon__color"
+            style={{ backgroundColor: color }}
+          />
+        ))}


이 부분 너무 깔끔하고 좋은거 같습니다!!

joonwonBaek · 2024-10-06T12:24:36Z

👍👍👍👍👍👍👍👍👍👍👍👍👍👍👍👍👍👍👍👍👍👍👍👍👍👍👍
👍👍👍👍👍👍👍👍👍👍👍👍👍👍👍👍👍👍👍👍👍👍👍👍👍👍👍
👍👍👍👍👍👍👍👍👍👍👍👍👍👍👍👍👍👍👍👍👍👍👍👍👍👍👍

seungineer

LGTM 🚀🚀🚀
theme selector 바깥 영역을 클릭하면, 창이 닫히는 것까지 포함되어 them selector 완성도가 높아져 더욱 좋은 것 같아요 👍👍

seungineer · 2024-10-06T12:32:45Z

packages/view/src/components/ThemeSelector/ThemeSelector.tsx

+    const handleClickOutside = (event: MouseEvent) => {
+      if (themeSelectorRef.current && !themeSelectorRef.current.contains(event.target as Node)) {
+        setIsOpen(false);
+      }
+    };
+    document.addEventListener("mousedown", handleClickOutside);
+    return () => {
+      document.removeEventListener("mousedown", handleClickOutside);
+    };
+  }, []);


(궁금💭)

if (themeSelectorRef.current && !themeSelectorRef.current.contains(event.target as Node))

해당 조건이 어떤 이유로 필요한지 궁금합니다! ?ㅅ?

해당 부분은 theme selector의 바깥 영역을 확인하는 코드인데요,
"mousedown" 이벤트가 발생해 handleClickOutside() 함수가 실행되면,

themeSelectorRef.current 코드를 통해 현재 시점에서 themeSelectorRef가 DOM에 있는지 확인하게 됩니다!

그리고 DOM에 있다면, !themeSelectorRef.current.contains(event.target as Node) 코드를 통해 현재 이벤트의 타겟인 클릭 지점이 theme selector의 영역 안에 있는지 확인합니다. 클릭 지점이 ref의 영역 안에 있으면 setIsOpen(false)를 실행하지 않아 창이 닫히지 않게 되고, 영역 밖에 있으면 닫히게 됩니다!!

xxxjinn · 2024-10-07T01:30:11Z

packages/view/src/components/ThemeSelector/ThemeSelector.tsx

-                title={theme.title}
-                value={theme.value}
-                colors={theme.colors}
+                {...theme}


👍👍👍👍👍👍👍

xxxjinn · 2024-10-07T01:31:28Z

packages/view/src/ide/IDEPort.ts

@@ -10,5 +10,5 @@ export default interface IDEPort {
  sendRefreshDataMessage: (payload?: string) => void;
  sendFetchAnalyzedDataMessage: (payload?: string) => void;
  sendFetchBranchListMessage: () => void;
-  setPrimaryColor: (color: string) => void;
+  setCustomTheme: (theme: string) => void;


사소한 부분이지만 실제 setCustomTheme 함수의 매개변수는 color라고 되어있는 것 같습니다!

setCustomTheme() 함수를 여러 군데에서 이름만 동일하게 사용하다보니 헷갈려 이런 문제가 생기네요 ... !

어느 부분에서 사용되는 함수인지 명시적으로 사용해야겠습니다 ............
찾아주셔서 감사합니다 !!!!!!!!!!! 👍👍👍👍👍👍

xxxjinn · 2024-10-07T01:33:04Z

packages/vscode/src/setting-repository.ts

+export const setTheme = async (newTheme: string) => {
  const configuration = vscode.workspace.getConfiguration();
-  configuration.update(SETTING_PROPERTY_NAMES.PRIMARY_COLOR, color);
+  configuration.update(SETTING_PROPERTY_NAMES.THEME, newTheme);
 };

-export const getPrimaryColor = (): string => {
+export const getTheme = async (): Promise<string> => {


(궁금) async를 왜 달아주셨는지 궁금합니다!

이 부분 트러블슈팅 하다가 configuration 가져오는 부분이 비동기인가 싶어서 추가했던 것 같습니다 .. !
지금 확인해보니 async처리 하지 않아도 잘 동작하네요 ㅎㅎ ...

다음 이슈에서 수정하도록 하겠습니다 !!

ytaek

view랑 vscode까지 다 작업하시느라 고생많으셨습니다.
저장하고 나니 theme 기능이 잘 동작하는 느낌이 더 팍팍나네요 👍👍👍

(사소)
전반적으로 customTheme과 theme이 같이 사용된 것 같은데,
혹시 다른 의미가 있는게 아니라면 하나로 통일시켜도 좋을 것 같습니다.

taboowiths · 2024-10-10T02:06:36Z

view랑 vscode까지 다 작업하시느라 고생많으셨습니다. 저장하고 나니 theme 기능이 잘 동작하는 느낌이 더 팍팍나네요 👍👍👍

(사소) 전반적으로 customTheme과 theme이 같이 사용된 것 같은데, 혹시 다른 의미가 있는게 아니라면 하나로 통일시켜도 좋을 것 같습니다.

theme 이라는 용어가 현재 ThemeSelector와 관련된 로직에서만 쓰이는 것으로 판단되니, 간단하게 theme 으로 통일하는 것이 좋을 것 같습니다 .

다음 issue로 올리겠습니다 !!!

customTheme -> theme으로 용어 통일
불필요한 비동기 제거
함수 역할에 기반한 함수명으로 변경

taboowiths added 6 commits October 5, 2024 11:13

feat: vscode configuration의 custom-theme 값을 window 전역 변수로 설정

f9821f3

feat: 함수/변수명 변경

c6bd614

feat: window 전역 변수에서 custom-theme 불러오도록 수정

c49f15d

refactor: themeInfo 타입 생성 및 지정

78e43a9

refactor: 변수명 변경 및 일부 로직 수정

e03075f

feat: ThemeSelector 영역 외 클릭 시 창 닫기

7bd6aa5

taboowiths added the ✨ view label Oct 6, 2024

taboowiths added this to the v0.7.2 milestone Oct 6, 2024

taboowiths self-assigned this Oct 6, 2024

taboowiths requested review from a team as code owners October 6, 2024 11:32

joonwonBaek approved these changes Oct 6, 2024

View reviewed changes

seungineer approved these changes Oct 6, 2024

View reviewed changes

xxxjinn reviewed Oct 7, 2024

View reviewed changes

ytaek approved these changes Oct 7, 2024

View reviewed changes

taboowiths merged commit 8f28b4e into githru:main Oct 10, 2024
2 checks passed

taboowiths mentioned this pull request Oct 13, 2024

[view] 브랜치 변경 시 테마 초기화되는 문제 해결 및 코드 리팩토링 #767

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[view] Theme Selector 테마 전역 저장 및 코드 리팩토링 #760

[view] Theme Selector 테마 전역 저장 및 코드 리팩토링 #760

taboowiths commented Oct 6, 2024

joonwonBaek left a comment

joonwonBaek Oct 6, 2024

joonwonBaek commented Oct 6, 2024

seungineer left a comment

seungineer Oct 6, 2024

taboowiths Oct 7, 2024 •

edited

Loading

xxxjinn Oct 7, 2024

xxxjinn Oct 7, 2024 •

edited

Loading

taboowiths Oct 7, 2024

xxxjinn Oct 7, 2024

taboowiths Oct 7, 2024

ytaek left a comment

taboowiths commented Oct 10, 2024 •

edited

Loading

[view] Theme Selector 테마 전역 저장 및 코드 리팩토링 #760

[view] Theme Selector 테마 전역 저장 및 코드 리팩토링 #760

Conversation

taboowiths commented Oct 6, 2024

Related issue

Result

Work list

1. Theme Selector 테마 전역 저장

2. 코드 리팩토링

Discussion

joonwonBaek left a comment

Choose a reason for hiding this comment

joonwonBaek Oct 6, 2024

Choose a reason for hiding this comment

joonwonBaek commented Oct 6, 2024

seungineer left a comment

Choose a reason for hiding this comment

seungineer Oct 6, 2024

Choose a reason for hiding this comment

taboowiths Oct 7, 2024 • edited Loading

Choose a reason for hiding this comment

xxxjinn Oct 7, 2024

Choose a reason for hiding this comment

xxxjinn Oct 7, 2024 • edited Loading

Choose a reason for hiding this comment

taboowiths Oct 7, 2024

Choose a reason for hiding this comment

xxxjinn Oct 7, 2024

Choose a reason for hiding this comment

taboowiths Oct 7, 2024

Choose a reason for hiding this comment

ytaek left a comment

Choose a reason for hiding this comment

taboowiths commented Oct 10, 2024 • edited Loading

taboowiths Oct 7, 2024 •

edited

Loading

xxxjinn Oct 7, 2024 •

edited

Loading

taboowiths commented Oct 10, 2024 •

edited

Loading