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

Hotfix reset css #7

Merged
merged 6 commits into from
Sep 13, 2024
Merged

Hotfix reset css #7

merged 6 commits into from
Sep 13, 2024

Conversation

dandamdandam
Copy link
Collaborator

@dandamdandam dandamdandam commented Sep 10, 2024

PR 설명

vanilla extract 특성 상 globalStyle로 뭔가 지정하고 불러오면, 그 설정이 계속 적용되는 것 같습니다. 이와 관련해서 몇가지 제안하려고 합니다.

  • 각자 css 작업까지 다하고 pr하면 문제가 일어날 수 있을 것 같아, globalStyle는 한 파일에서 관리하는게 어떨까 제안합니다. -> globalStyle.css.ts
  • **(reset css)**저희 디자인 보면 default style(ex) h1의 마진과 패딩 등)을 그대로 사용하는게 거의 없습니다. 초기화 하는게 편할 것 같습니다. -> 재연님이 관련해서 작성해뒀길래 가져오고, 제가 몇가지 더 추가했습니다.
  • 하는 김에 색이랑 폰트 variable들 추가했습니다.
  • 구글 material icon도 가져올 수 있게 추가했습니다.

관련 내용 hotfix-resetcss에 추가해놓을테니 확인해주세요.

vanilla extract 처음 써봐서 이렇게 쓰는게 맞는지 잘 모르겠습니다. 비판적으로 봐주세요.

Copy link

vercel bot commented Sep 10, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
knuland ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 13, 2024 2:54am

@Kim-jaeyeon
Copy link
Collaborator

Kim-jaeyeon commented Sep 10, 2024

좋은 것 같아요! 저는 현재 각 페이지마다 css.ts에 global스타일을 적용해왔었는데, 한 파일에서 global스타일을 관리하는 게 나을 수도 있을 것 같네용

@jjh4450
Copy link
Collaborator

jjh4450 commented Sep 10, 2024

일단 제가 기획하던 세팅은 style 폴더를 만들고 아래 두 파일을 제작하는 것이었습니다.

style
| style.css.ts
| sprinkles.css.ts

sprinkles.css.ts

import { defineProperties, createSprinkles } from '@vanilla-extract/sprinkles';

const colors = {
    primary: '#3498db',
    secondary: '#2ecc71',
    danger: '#e74c3c',
    background: '#ecf0f1',
    text: '#2c3e50',
};

const space = {
    none: '0',
    small: '4px',
    medium: '8px',
    large: '16px',
    xlarge: '32px',
};

const responsiveProperties = defineProperties({
    conditions: {
        mobile: {},
        tablet: { '@media': 'screen and (min-width: 768px)' },
        desktop: { '@media': 'screen and (min-width: 1024px)' },
    },
    defaultCondition: 'mobile',
    properties: {
        margin: space,
        padding: space,
        color: colors,
        backgroundColor: colors,
    },
});

export const sprinkles = createSprinkles(responsiveProperties);

// 타입 추론을 돕기 위한 export
export type Sprinkles = Parameters<typeof sprinkles>[0];

sprinkles는 아직 연구중입니다.

@jjh4450
Copy link
Collaborator

jjh4450 commented Sep 10, 2024

일단 제가 기획하던 세팅은 style 폴더를 만들고 아래 두 파일을 제작하는 것이었습니다.

style
| style.css.ts
| sprinkles.css.ts

sprinkles.css.ts

import { defineProperties, createSprinkles } from '@vanilla-extract/sprinkles';

const colors = {
    primary: '#3498db',
    secondary: '#2ecc71',
    danger: '#e74c3c',
    background: '#ecf0f1',
    text: '#2c3e50',
};

const space = {
    none: '0',
    small: '4px',
    medium: '8px',
    large: '16px',
    xlarge: '32px',
};

const responsiveProperties = defineProperties({
    conditions: {
        mobile: {},
        tablet: { '@media': 'screen and (min-width: 768px)' },
        desktop: { '@media': 'screen and (min-width: 1024px)' },
    },
    defaultCondition: 'mobile',
    properties: {
        margin: space,
        padding: space,
        color: colors,
        backgroundColor: colors,
    },
});

export const sprinkles = createSprinkles(responsiveProperties);

// 타입 추론을 돕기 위한 export
export type Sprinkles = Parameters<typeof sprinkles>[0];

sprinkles는 아직 연구중입니다.

shared 이용하는 게 적절할까요? 그렇다면 어떻게 구성하는 게 좋을까요, style 폴더를 만들고 넣기? 아니면 .css.ts 파일만 넣기?

@dandamdandam
Copy link
Collaborator Author

dandamdandam commented Sep 10, 2024

저도 비슷하게 구성했습니다.
shared/style

  • globalStyle.css.ts -> reset css관련
  • vars.css.ts -> 변수, 폰트 정의

shared에 작성한 이유는 style 폴더를 작성해봤자 colocation 형식으로 파일 배치하면 거의 안쓰일 것 같아 따로 넣었습니다. 말그대로 공유되는 무언가기도 하고요.

저도 그냥 reset.css 임포트 하는걸 좋아하긴 하지만.. 뭔가 한분이 이미 작업을 많이 한 것 같기도 해서 제가 직접 작성했고, 변경할 수 있는 글로벌 설정 파일 있으면 좋을 것 같아 추가했습니다.

@dandamdandam
Copy link
Collaborator Author

@jjh4450
만약 sprinkles를 사용해 지정한 변수를 사용한다면 이런 식으로 쓰면 되는 건가요?

// src/pages/main/index.css.ts
import { style } from "@vanilla-extract/css";
import { sprinkles } from "../../shard/sprinkles.css.ts"; // sprinkles 가져오기

export const h1Style = style([
    sprinkles({
        color: 'danger', // danger 색상을 sprinkles로 지정
    }),
]);

@jjh4450
Copy link
Collaborator

jjh4450 commented Sep 12, 2024

@jjh4450 만약 sprinkles를 사용해 지정한 변수를 사용한다면 이런 식으로 쓰면 되는 건가요?

// src/pages/main/index.css.ts
import { style } from "@vanilla-extract/css";
import { sprinkles } from "../../shard/sprinkles.css.ts"; // sprinkles 가져오기

export const h1Style = style([
    sprinkles({
        color: 'danger', // danger 색상을 sprinkles로 지정
    }),
]);
export const headerStyles = style([
    sprinkles({
        backgroundColor: 'background',
        padding: {
            mobile: 'medium',
            tablet: 'large', // <-- 적용 O (sprinkles 내부에 있기 때문)
        },
    }),
    {
        display: 'flex',
        justifyContent: 'space-between',
        alignItems: 'center',
        '@media': {
            'screen and (max-width: 600px)': {
                flexDirection: 'column',
                alignItems: 'flex-start',
               //  padding: 'xlarge', // <-- xlarge는 적용 X (sprinkles 외부에 있기 때문)
            },
        },
    },
]);

위 코드에서 backgroundColor와 padding은 미리 정의된 sprinkles 속성을 활용하고 있습니다. 이 방식은 일관된 스타일 유지와 반응형 디자인 구현에 효과적이라고 생각됩니다.
그러나, sprinkles의 특성상 사전에 정의되지 않은 속성은 사용할 수 없다는 문제가 있습니다. 이를 고려하면, 우선 CSS 변수로 기본적인 스타일을 구성한 후, 자주 사용되는 항목에만 sprinkles를 적용하는 것이 더 적절할 것입니다.

이러한 맥락에서 볼 때, sprinkles의 도입은 현재 고려해야 할 우선순위는 아닐 것 같습니다! 차라리 var 부분에 대한 구현에 집중을 하는게 맞다고 생각됩니다.

@dandamdandam
Copy link
Collaborator Author

그럼 글로벌설정에 이견 없는걸로 알고, 리뷰 받으면 머지하겠습니다!
@flareseek @jjh4450

@flareseek
Copy link
Owner

@jjh4450 준환님 확인하시고 이상없으면 머지해주세요!

@dandamdandam
Copy link
Collaborator Author

dandamdandam commented Sep 12, 2024

cc53ff6 내용 설명~

css에 이미지 추가하려니까 url은 변환되는데 에셋이 안만들어져서 not found 에러가 뜨더라고요.
찾아보니까 아직 해결안된 문제인 것 같아서 -> JonasKruckenberg/imagetools#563

일단 layout에서 배경이미지를 랜더링해서 에셋소스를 만들게하고, css 파일에서 사용하도록하는 야매방식으로 고쳤습니다.
다른 좋은 방법 생각나시면 고쳐주시고, 아니면 그냥 머지해주세요

@dandamdandam dandamdandam requested review from jjh4450 and removed request for jjh4450 September 13, 2024 02:31
jjh4450
jjh4450 previously approved these changes Sep 13, 2024
src/shared/styles/globalStyle.css.ts Show resolved Hide resolved
@dandamdandam dandamdandam merged commit 35db357 into develop Sep 13, 2024
3 checks passed
@dandamdandam dandamdandam deleted the hotfix-resetcss branch September 13, 2024 02:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question 질문있어요
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants