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

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

Merged
merged 8 commits into from
Oct 17, 2024
3 changes: 2 additions & 1 deletion packages/view/src/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { RefreshButton } from "components/RefreshButton";
import type { IDESentEvents } from "types/IDESentEvents";
import type { RemoteGitHubInfo } from "types/RemoteGitHubInfo";
import { useBranchStore, useDataStore, useLoadingStore, useOwnerStore, useRepoStore } from "store";
import { THEME_INFO } from "components/ThemeSelector/ThemeSelector.const";

const App = () => {
const initRef = useRef<boolean>(false);
Expand Down Expand Up @@ -53,7 +54,7 @@ const App = () => {
if (loading) {
return (
<BounceLoader
color="#ff8272"
color={THEME_INFO[window.theme as keyof typeof THEME_INFO].colors.primary}
Copy link
Member

Choose a reason for hiding this comment

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

로딩 컴포넌트에도 선택한 theme 컬러가 적용되니 좋네요 👍👍

loading={loading}
cssOverride={{
position: "fixed",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
import type { ThemeInfo } from "./ThemeSelector.type";

export const THEME_INFO: ThemeInfo = {
Comment on lines +1 to +3
Copy link
Contributor

Choose a reason for hiding this comment

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

분리 설계 좋습니다 👍

githru: {
title: "Githru",
value: "githru",
colors: {
primary: "#e06091",
secondary: "#8840bb",
tertiary: "#ffd08a",
},
},
"hacker-blue": {
title: "Hacker Blue",
value: "hacker-blue",
colors: {
primary: "#456cf7",
secondary: "#3f4c73",
tertiary: "#6c60f0",
},
},
aqua: {
title: "Aqua",
value: "aqua",
colors: {
primary: "#51decd",
secondary: "#0687a3",
tertiary: "#a7ffff",
},
},
"cotton-candy": {
title: "Cotton Candy",
value: "cotton-candy",
colors: {
primary: "#ffcccb",
secondary: "#feffd1",
tertiary: "#a39aeb",
},
},
mono: {
title: "Mono",
value: "mono",
colors: {
primary: "#68788f",
secondary: "#3a4776",
tertiary: "#9aaed1",
},
},
};
82 changes: 10 additions & 72 deletions packages/view/src/components/ThemeSelector/ThemeSelector.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,82 +3,19 @@ import "./ThemeSelector.scss";
import AutoAwesomeIcon from "@mui/icons-material/AutoAwesome";
import CloseIcon from "@mui/icons-material/Close";

import { setCustomTheme } from "services";
import { sendUpdateThemeCommand } from "services";

type ThemeInfo = {
title: string;
value: string;
colors: {
primary: string;
secondary: string;
tertiary: string;
};
};
import { THEME_INFO } from "./ThemeSelector.const";
import type { ThemeInfo } from "./ThemeSelector.type";

type ThemeIconsProps = ThemeInfo & {
type ThemeIconsProps = ThemeInfo[keyof ThemeInfo] & {
onClick: () => void;
};

const themeInfo: ThemeInfo[] = [
{
title: "Githru",
value: "githru",
colors: {
primary: "#e06091",
secondary: "#8840bb",
tertiary: "#ffd08a",
},
},
{
title: "Hacker Blue",
value: "hacker-blue",
colors: {
primary: "#456cf7",
secondary: "#3f4c73",
tertiary: "#6c60f0",
},
},
{
title: "Aqua",
value: "aqua",
colors: {
primary: "#51decd",
secondary: "#0687a3",
tertiary: "#a7ffff",
},
},
{
title: "Cotton Candy",
value: "cotton-candy",
colors: {
primary: "#ffcccb",
secondary: "#feffd1",
tertiary: "#a39aeb",
},
},

{
title: "Mono",
value: "mono",
colors: {
primary: "#68788f",
secondary: "#3a4776",
tertiary: "#9aaed1",
},
},
];

const ThemeIcons = ({ title, value, colors, onClick }: ThemeIconsProps) => {
const [selectedItem, setSelectedItem] = useState<string>("");

useEffect(() => {
const selectedTheme = document.documentElement.getAttribute("custom-type");
if (selectedTheme) setSelectedItem(selectedTheme);
}, []);

return (
<div
className={`theme-icon${selectedItem === value ? "--selected" : ""}`}
className={`theme-icon${window.theme === value ? "--selected" : ""}`}
onClick={onClick}
role="presentation"
>
Expand All @@ -101,8 +38,9 @@ const ThemeSelector = () => {
const themeSelectorRef = useRef<HTMLDivElement>(null);

const handleTheme = (value: string) => {
setCustomTheme(value);
document.documentElement.setAttribute("custom-type", value);
sendUpdateThemeCommand(value);
window.theme = value;
document.documentElement.setAttribute("theme", value);
Comment on lines +41 to +43
Copy link
Member

Choose a reason for hiding this comment

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

Discussion에 써주신대로 window.theme을 상태로 대신할 수도 있겠네요 😲

};

useEffect(() => {
Expand All @@ -118,7 +56,7 @@ const ThemeSelector = () => {
}, []);

useEffect(() => {
document.documentElement.setAttribute("custom-type", window.theme);
document.documentElement.setAttribute("theme", window.theme);
}, []);

return (
Expand All @@ -137,7 +75,7 @@ const ThemeSelector = () => {
/>
</div>
<div className="theme-selector__list">
{themeInfo.map((theme) => (
{Object.entries(THEME_INFO).map(([_, theme]) => (
<ThemeIcons
key={theme.value}
{...theme}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
export type ThemeInfo = {
[key in "githru" | "hacker-blue" | "aqua" | "cotton-candy" | "mono"]: {
Copy link
Contributor

Choose a reason for hiding this comment

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

무결성을 위해 key의 literal type을 강제한 것 매우 좋아보입니다!
추가로 theme key list를 별도로 빼도 괜찮을 것 같습니다!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

넵 그럼 다음 이슈에서 상수로 관리하도록 빼두겠습니다 ! ! !

title: string;
value: key;
colors: {
primary: string;
secondary: string;
tertiary: string;
};
};
};
8 changes: 4 additions & 4 deletions packages/view/src/ide/FakeIDEAdapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Copy link
Member

Choose a reason for hiding this comment

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

함수명이 훨씬 직관적이네요 😮

sessionStorage.setItem("THEME", theme);
const message: IDEMessage = {
command: "updateCustomTheme",
command: "updateTheme",
};
this.sendMessageToMe(message);
}
Expand All @@ -76,7 +76,7 @@ export default class FakeIDEAdapter implements IDEPort {
command,
payload: JSON.stringify(fakeBranchList),
};
case "updateCustomTheme":
case "updateTheme":
return {
command,
payload: sessionStorage.getItem("CUSTOM_THEME") as string,
Expand Down
2 changes: 1 addition & 1 deletion packages/view/src/ide/IDEPort.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,5 +10,5 @@ export default interface IDEPort {
sendRefreshDataMessage: (payload?: string) => void;
sendFetchAnalyzedDataMessage: (payload?: string) => void;
sendFetchBranchListMessage: () => void;
setCustomTheme: (theme: string) => void;
sendUpdateThemeMessage: (theme: string) => void;
}
4 changes: 2 additions & 2 deletions packages/view/src/ide/VSCodeIDEAdapter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,9 @@ export default class VSCodeIDEAdapter implements IDEPort {
this.sendMessageToIDE(message);
}

public setCustomTheme(theme: string) {
public sendUpdateThemeMessage(theme: string) {
Copy link
Contributor

Choose a reason for hiding this comment

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

함수명 적절하게 잘 바꿔주신 것 같아요!!!🥰🥰

const message: IDEMessage = {
command: "updateCustomTheme",
command: "updateTheme",
payload: JSON.stringify({ theme }),
};
this.sendMessageToIDE(message);
Expand Down
4 changes: 2 additions & 2 deletions packages/view/src/services/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@ import { container } from "tsyringe";

import type IDEPort from "ide/IDEPort";

export const setCustomTheme = (color: string) => {
export const sendUpdateThemeCommand = (theme: string) => {
const ideAdapter = container.resolve<IDEPort>("IDEAdapter");
ideAdapter.setCustomTheme(color);
ideAdapter.sendUpdateThemeMessage(theme);
};

export const sendFetchAnalyzedDataCommand = (selectedBranch?: string) => {
Expand Down
10 changes: 5 additions & 5 deletions packages/view/src/styles/_colors.scss
Original file line number Diff line number Diff line change
Expand Up @@ -5,39 +5,39 @@ $color-medium-gray: #757880;
$color-dark-gray: #3c4048;
$color-background: #222324;

html[custom-type="githru"] {
html[theme="githru"] {
--color-primary: #e06091;
--color-secondary: #8840bb;
--color-tertiary: #ffd08a;
--color-success: #07bebe;
--color-failed: #ee2479;
}

html[custom-type="hacker-blue"] {
html[theme="hacker-blue"] {
--color-primary: #456cf7;
--color-secondary: #3f4c73;
--color-tertiary: #6c60f0;
--color-success: #1fc7a9;
--color-failed: #ee2479;
}

html[custom-type="aqua"] {
html[theme="aqua"] {
--color-primary: #51decd;
--color-secondary: #0687a3;
--color-tertiary: #a7ffff;
--color-success: #008cde;
--color-failed: #ee2479;
}

html[custom-type="cotton-candy"] {
html[theme="cotton-candy"] {
--color-primary: #ffcccb;
--color-secondary: #feffd1;
--color-tertiary: #a39aeb;
--color-success: #7ad5c4;
--color-failed: #ff8bbc;
}

html[custom-type="mono"] {
html[theme="mono"] {
--color-primary: #68788f;
--color-secondary: #3a4776;
--color-tertiary: #9aaed1;
Expand Down
2 changes: 1 addition & 1 deletion packages/view/src/types/IDEMessage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,4 @@ export type IDEMessageCommandNames =
| "fetchAnalyzedData"
| "fetchBranchList"
| "fetchCurrentBranch"
| "updateCustomTheme";
| "updateTheme";
8 changes: 4 additions & 4 deletions packages/vscode/src/setting-repository.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,17 +17,17 @@ export const deleteGithubToken = async (secrets: vscode.SecretStorage) => {
return await secrets.delete(SETTING_PROPERTY_NAMES.GITHUB_TOKEN);
};

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

export const getTheme = async (): Promise<string> => {
export const getTheme = () => {
const configuration = vscode.workspace.getConfiguration();
const theme = configuration.get(SETTING_PROPERTY_NAMES.THEME) as string;

if (!theme) {
await setTheme("githru");
setTheme("githru");
return "githru";
}
return theme;
Expand Down
20 changes: 7 additions & 13 deletions packages/vscode/src/webview-loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,13 @@

const icon_path = vscode.Uri.file(path.join(this.extensionPath, "images", "logo.png"));
this._panel.iconPath = icon_path;
let analyzedData;

Check warning on line 34 in packages/vscode/src/webview-loader.ts

View workflow job for this annotation

GitHub Actions / build (20.x)

'analyzedData' is defined but never used
this._panel.webview.onDidReceiveMessage(async (message: { command: string; payload?: string }) => {
try {
const { command, payload } = message;

if (command === "fetchAnalyzedData" || command === "refresh") {
const baseBranchName = (payload && JSON.parse(payload)) ?? (await fetchCurrentBranch());

Check warning on line 40 in packages/vscode/src/webview-loader.ts

View workflow job for this annotation

GitHub Actions / build (20.x)

'baseBranchName' is assigned a value but never used
try {
const baseBranchName = (payload && JSON.parse(payload)) ?? (await fetchCurrentBranch());
const storedAnalyzedData = context.workspaceState.get<ClusterNode[]>(
Expand Down Expand Up @@ -77,10 +77,10 @@
});
}

if (command === "updateCustomTheme") {
const colorCode = payload && JSON.parse(payload);
if (colorCode.theme) {
setTheme(colorCode.theme);
if (command === "updateTheme") {
const themeInfo = payload && JSON.parse(payload);
if (themeInfo.theme) {
setTheme(themeInfo.theme);
}
}
} catch (e) {
Expand All @@ -92,7 +92,7 @@
// this.dispose();
// throw new Error("Project not connected to Git.");
// }
this.setWebviewContent();
this._panel.webview.html = this.getWebviewContent(this._panel.webview);
}

dispose() {
Expand All @@ -111,12 +111,12 @@
});
}

private async getWebviewContent(webview: vscode.Webview): Promise<string> {
Copy link
Contributor

Choose a reason for hiding this comment

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

(궁금) return type은 왜 삭제하게 되셨는지 궁금합니다.!!

Copy link
Contributor Author

@taboowiths taboowiths Oct 16, 2024

Choose a reason for hiding this comment

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

기존 함수를 async로 변경하고, Promise Type을 씌웠는데, 다시 동기 함수로 변경하는 과정에서 삭제가 되어버렸습니다 🥲
의도한 것은 아니고 기존 코드를 확인 못하고 제거한 부분인데, 일단 해당 함수가 반환하는 변수명이 returnString으로 string 타입을 반환한다는 부분이 명시적이고, 타입 추론으로 인해 코드상 에러는 없으니, 다음 이슈에 반환 타입을 기재하도록 하겠습니다 ㅎㅎ

private getWebviewContent(webview: vscode.Webview) {
const reactAppPathOnDisk = vscode.Uri.file(path.join(this.extensionPath, "dist", "webviewApp.js"));
const reactAppUri = webview.asWebviewUri(reactAppPathOnDisk);
// const reactAppUri = reactAppPathOnDisk.with({ scheme: "vscode-resource" });

const theme = await getTheme();
const theme = getTheme();
const returnString = `
<!DOCTYPE html>
<html lang="en">
Expand All @@ -141,12 +141,6 @@
return returnString;
}

private async setWebviewContent() {
if (this._panel) {
this._panel.webview.html = await this.getWebviewContent(this._panel.webview);
}
}

public setGlobalOwnerAndRepo(owner: string, repo: string) {
if (this._panel) {
this._panel.webview.postMessage({
Expand Down
Loading