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

영감 보기 그리드 디자인 설정 #35

Merged
merged 6 commits into from
Apr 13, 2022
Merged

영감 보기 그리드 디자인 설정 #35

merged 6 commits into from
Apr 13, 2022

Conversation

positiveko
Copy link
Member

#31

⛳️작업 내용

design: content thumbnail 디자인 작성 및 공통컴포넌트 tag 추가

아직 디자인 미완성이지만, globalStyle에 수정이 있어서 풀리퀘 올립니다.

  • 피그마 와이어프레임 단계라 theme을 크게 수정하지는 않았어요. 나중에 디자인 컨셉 나오게 되면 다시 theme에 font-size, font-weight, color 추가해서 적용하면 될 것 같아요 :)
  • 반복적으로 하드 코딩 된 부분은 백엔드 분이랑 이야기 후에 목데이터로 변경해서 map 돌리는 방식으로 수정해놓겠습니다!

design: global styles reset 추가 및 reset 변경에 따른 디자인 세부 수정

  • globalStyle에 reset을 넣어놨어요. 평소에 하던 방식이긴 한데.. 다른 분들은 어떻게 하시나 궁금하네요 :) 수정 필요하다면 말씀해주세요!
  • 전체 body에 드래그 방지를 넣어놨는데, 클래스명으로 'draggable'을 주어 선택적으로 드래그하도록 했어요
  • 외려 반대로 드래그 자유도를 주되 제한된 부분에서만 드래그 방지하는 방법으로 변경 해야되나 고민이 되는데 이후 디자인 보면서 수정해도 될 것 같아요.
  body {
    -webkit-user-select: none;
    -moz-user-select: none;
    -ms-user-select: none;
    user-select: none;

    .draggable {
      -webkit-user-select: all;
      -moz-user-select: all;
      -ms-user-select: all;
      user-select: all;
    }
  }

📸스크린샷

image

📎레퍼런스

@positiveko positiveko added feat 새로운 기능 추가 style CSS 및 UI 디자인 변경 labels Apr 12, 2022
@positiveko positiveko added this to the Sprint 1 milestone Apr 12, 2022
@positiveko positiveko self-assigned this Apr 12, 2022
Copy link
Member

@hyesungoh hyesungoh left a comment

Choose a reason for hiding this comment

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

늦은 시간까지 고생하셨어요 👍 👍 👍 👍 👍 👍 👍

개인적인 의견으로는 css-in-js를 사용할 때는 최대한 간략하게 css 명을 짓는 편인데,

<article css={articleCss}>
  <section css={contentCss}>
    <p css={pragraphCss}>
      콘텐츠의 텍스트는 다음과 같이 삽입됩니다. 콘텐츠의 텍스트는 다음과 같이 삽입됩니다.
      콘텐츠의 텍스트는 다음과 같이 삽입됩니다. 콘텐츠의 텍스트는 다음과 같이 삽입됩니다.
    </p>
  </section>

  <section css={tagWrapperCss}>
    <Tag text="영감" />
    <Tag text="UX/UI" />
    <Tag text="브랜드 디자인 레퍼런스 모음집" />
    <Tag text="디자인인사이트" />
  </section>
</article>

예를 들면 위처럼이요! 다른 분들이 보시기에는 어떤 게 조금 더 보기 좋은 지,
혹은 개발할 때 편하신 지 의견 나눠보면 좋을 거 같아요!!

먼저 작업해주시고 감사합니다 !!! 💯 🥇

디자인 나오기 전에 먼저 작업해주신 거니까, 아래 코멘트는 제가 평소에하는 방법 정도로만 생각해주세요!!!! 절대 적용을 부탁드리는 건 아닙니당 👍

font-size: 10px;

:not(:last-child) {
margin-right: 8px;
Copy link
Member

Choose a reason for hiding this comment

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

해당 부분은 margin 보다는 부모 요소의 gap 속성을 이용하는 건 어떨까요!! 개인적인 의견입니다 😄

Copy link
Collaborator

@sensecodevalue sensecodevalue Apr 13, 2022

Choose a reason for hiding this comment

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

flex gap이 최신 safari에서는 잘되는데 14이전? 버전에는 잘 동작안하는걸로 알고 있어요!
지금은 15라 그냥 해도 될꺼 같다고 생각해요!

Copy link
Member Author

Choose a reason for hiding this comment

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

미팅에서 이야기한대로 뒤에 Css를 붙이되 이름은 자유롭게 정하는 것으로 하겠습니다 :)

Copy link
Member Author

Choose a reason for hiding this comment

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

gap으로 수정할게요 감사합니다 👍🏼


export default function ContentHeader() {
return (
<div css={ContentHeaderCss}>
Copy link
Member

Choose a reason for hiding this comment

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

div 보다 header 태그가 더 시맨틱할 것 같아요!

Copy link
Member Author

Choose a reason for hiding this comment

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

header로 수정했습니다 :) 💯


export type ContentThumbnailType = 'image' | 'text' | 'link';

export interface IContentThumbnailProps {
Copy link
Member

Choose a reason for hiding this comment

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

저희 컨벤션은 interface 앞에 I를 붙히지 않기로 했었습니다 ....!!
제가 린트 룰 업데이트해볼게요!!

Copy link
Member Author

Choose a reason for hiding this comment

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

ContentThumbnailProps로 수정했습니다 :)


export default function ContentThumbnail() {
return (
<div css={contentThumbnailBoxCss}>
Copy link
Member

Choose a reason for hiding this comment

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

각각의 영감들이니, div 보다 article 태그가 더 시맨틱할 것 같아요!!
그리고 해당 요소들을 감싸는 articlemain 혹은 section 태그로 상황에 따라 사용하는 것이 어떨까요??

Copy link
Member Author

Choose a reason for hiding this comment

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

고민이 되었던 부분인데 일단 section으로 수정했어요.
Home이라는 article 안에 header와 여러 개의 썸네일 section으로 구성되도록 수정했습니다 :) 피드백 감사해요!

font-size: 12px;
line-height: 18px;
letter-spacing: -0.01em;
word-wrap: break-word;
Copy link
Member

Choose a reason for hiding this comment

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

word-wrap, word-break 속성은 저는 주로 전역에서 선언했는데 의견이 궁금합니다 !

주로 단어가 안끊기는 것이 이쁘고, 영역이 넘어가면 자르게 하기 위해 아래와 같이 쓰곤 했어요 !

* {
word-break: keep-all;
word-wrap: break-word;
}

Copy link
Member Author

Choose a reason for hiding this comment

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

전역에서 선언하는 걸로 넣어놓을게요!

`;

const contentThumbnailCss = css`
position: absolute;
Copy link
Member

Choose a reason for hiding this comment

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

해당 이미지, 글, 링크 등이 overflow되지 않기 위해 position을 absolute로 주신 것으로 이해를 했는데 혹시 맞을 지 궁금합니다!

그리고 absolute이기 때문에, 크기를 확보하기 위해 ::after > padding-bottom 값을 주신 것으로 이해했어요!

해당 부분이 자칫하면 읽기 어렵거나 수정이 힘들 수도 있다고 생각해서,

export default function ContentThumbnail() {
  return (
    <article css={contentThumbnailBoxCss}>
      <section css={contentThumbnailCss}>
        <p css={contentText}>
          콘텐츠의 텍스트는 다음과 같이 삽입됩니다. 콘텐츠의 텍스트는 다음과 같이 삽입됩니다.
          콘텐츠의 텍스트는 다음과 같이 삽입됩니다. 콘텐츠의 텍스트는 다음과 같이 삽입됩니다.
        </p>
      </section>

      <section css={contentTagsCss}>
        <Tag text="영감" />
        <Tag text="UX/UI" />
        <Tag text="브랜드 디자인 레퍼런스 모음집" />
        <Tag text="디자인인사이트" />
      </section>
    </article>
  );
}

const contentThumbnailPadding = '12px';
const contentTagHeight = '40px';

const contentThumbnailBoxCss = (theme: Theme) => css`
  width: 100%;
  height: 100%;
  /* 디자인 상 최소 높이가 존재할 때, 혹은 전체 높이 고정일 시 height 속성으로 대입 */
  min-height: 300px;
  padding: ${contentThumbnailPadding};
  background: ${theme.color.gray};
  border-radius: 4px;

  display: flex;
  flex-direction: column;
  justify-content: space-between;
`;

const contentThumbnailCss = css`
  width: 100%;
  height: 100%;

  /* 예를 들어 이미지라면 */
  & > img {
    width: 100%;
    height: 100%;
    overflow: hidden;
    object-fit: cover;
  }
`;

const contentText = css`
  background: #fbfbfb;
  font-weight: 500;
  font-size: 12px;
  line-height: 18px;
  letter-spacing: -0.01em;
  word-wrap: break-word;
`;

const contentTagsCss = css`
  /* contentThumbnailCss 에서 100 100을 줬기 때문 */
  flex-shrink: 0;

  display: flex;
  align-items: center;
  height: ${contentTagHeight};
  overflow-x: scroll;
  -ms-overflow-style: none;

  ::-webkit-scrollbar {
    display: none !important;
  }
`;

이런 방식으로는 어떠실지 의견이 궁금해요 !!

물론 제가 이해를 못한 이유 때문에 적용하셨던 것일 수도 있으시고, 제가 많이 부족해서 모르는 부분이 있었을 수도 있으니
꼭 적용하실 필요는 없고 개인적인 의견입니다......!!

Copy link
Member Author

Choose a reason for hiding this comment

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

단순히 정사각형 그리드를 만들기 위해서였는데, aspect-ratio 프로퍼티를 제가 몰랐네요 😂
position과 after 가상 선택자는 수정해서 다시 올립니다! 감사합니다!

@hyesungoh
Copy link
Member

저는 reset할 시는

* {
  all: unset;
}

후에 개발하면서 필요한 button cursor 등과 같은 공통적인 것만 추가해주는 편이긴해요!!

현재 작업해주신 것도 매우 좋아요 👍 👍

reset.ts와 같이 모듈화해도 좋을 거 같아요!

Copy link
Collaborator

@ddarkr ddarkr left a comment

Choose a reason for hiding this comment

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

👍👍👍

저도 일부 마진 요소들을 flex 요소에선 gap으로 대체할 수 있을 것 같다는 생각이에요!

Copy link
Collaborator

@sensecodevalue sensecodevalue left a comment

Choose a reason for hiding this comment

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

감사합니다!! 좋아요!

Comment on lines 76 to 80
-ms-overflow-style: none;

::-webkit-scrollbar {
display: none !important;
}
Copy link
Collaborator

@sensecodevalue sensecodevalue Apr 13, 2022

Choose a reason for hiding this comment

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

웹앱인 만큼 전역에서 다 제거하는게 편할 수 도 있을것 같아요! (스크롤 보여주는 경우가 없다면!)

Copy link
Member Author

Choose a reason for hiding this comment

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

좋은데요! 글로벌 스타일에 추가해 놓을게요 :)

font-size: 10px;

:not(:last-child) {
margin-right: 8px;
Copy link
Collaborator

@sensecodevalue sensecodevalue Apr 13, 2022

Choose a reason for hiding this comment

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

flex gap이 최신 safari에서는 잘되는데 14이전? 버전에는 잘 동작안하는걸로 알고 있어요!
지금은 15라 그냥 해도 될꺼 같다고 생각해요!

@positiveko positiveko linked an issue Apr 13, 2022 that may be closed by this pull request
@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented Apr 13, 2022

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 5d35e24
Status: ✅  Deploy successful!
Preview URL: https://f3c46f4b.ygt.pages.dev

View logs

@positiveko
Copy link
Member Author

혜성님 요거를 적용해보려고 했더니.. 거의 resetCss에 있는 것들을 다 적용해주어야 할 정도로 많이 깨지더라구요 !
이 부분 다음 풀리퀘에서 다시 한번 봐보는 걸로 하고 일단 resetCss 모듈화 하는 것으로 끝낼게요!

@positiveko positiveko merged commit d0c65f1 into main Apr 13, 2022
@positiveko positiveko deleted the issue/31 branch April 13, 2022 15:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat 새로운 기능 추가 style CSS 및 UI 디자인 변경
Projects
None yet
Development

Successfully merging this pull request may close these issues.

영감 보기 - 전체 보기(그리드)
4 participants