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

[FE] Filter Menu Ui #30

Closed
wants to merge 20 commits into from
Closed

[FE] Filter Menu Ui #30

wants to merge 20 commits into from

Conversation

Elllin
Copy link
Contributor

@Elllin Elllin commented Jun 15, 2020

개요

  • Filter Menu Ui

작업사항

  • material-ui-popup-state 설치
  • GlobalStyle 변수 추가, 폰트 크기 변경
  • app.js에서 header 제외 mainWrap으로 감싸서 width 조정
    (header 제외하고는 width가 100%가 아닌 것 같아 추가했습니다.)
  • filter 버튼 추가
  • menuList 컴포넌트를 새로 만들었는데 이슈 목록 filter에서도 재사용 가능할 것 같아 common 폴더에 넣어놨습니다. 아래 사용방법 기록해 놨으니 참고해서 사용해 주세요

MenuList 컴포넌트 사용방법

  • props는 text, title을 받습니다

  • 사용 용도는 두 가지가 있습니다

    1. title로써 상단에 사용
    2. menuList로써 hover와 마우스 클릭이 되는 list로 사용
  • title로 사용 시 : props로 title을 사용하고 title에 들어갈 text를 넣어주면 됩니다.

  (예) <MenuList title="Filter lssues" />
  • menuList로 사용 시 : props로 text를 사용하고 list에 들어갈 text를 넣어주면 됩니다.
  (예) <MenuList text="Open issues" />  

변경로직

  • 전체 font-size

변경전

 font-size: 1rem;

변경후

font-size: 14px;

기타

  • material-ui 사용법이 익숙지 않아 menuList가 아직 지저분하고 재사용이 얼마나 될지는 모르겠지만 우선 공통 컴포넌트에 추가해 두었습니다.
  • font size나 app에서 mainWrap는 변경하셔도 됩니다.

closed #10

@Elllin Elllin added the 🍺 FE 프론트엔드 이슈 label Jun 15, 2020
@Elllin Elllin added this to the FE WEEK 1 milestone Jun 15, 2020
@Elllin Elllin changed the title Fe/feature/filter menu [FE] Filter Menu Ui Jun 15, 2020
@Elllin Elllin linked an issue Jun 15, 2020 that may be closed by this pull request
4 tasks
FE/src/App.js Outdated

import GlobalStyle from "./style/GlobalStyle";
import styled from "styled-components";
Copy link
Contributor

Choose a reason for hiding this comment

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

제 생각에는, 이번 프로젝트에서 styled-components의 사용은 할 수 있으면 하지 않는 게 좋은 방법같아요. 익숙치 않고 어렵겠지만,(저도 그래요) MaterialUI를 사용하는 만큼, 커스텀 스타일링도 라이브러리가 제공하는 API에 따라서 해보려고 합니다.

필요하다면 MaterialUI에서 제공하는 @material-ui/styles를 통해 스타일링 해보려고 해요.

엘린님은 어떻게 생각하세요?

Copy link
Contributor

Choose a reason for hiding this comment

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

styled-components 와 함께 사용하는 방식을 정리해놓은 MaterialUI의 문서와 블로그도 있네요. 혹시 필요하시다면...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

네 좋습니다! styled-components는 최대한 안 쓰는 쪽으로 하겠습니다. 하지만 MaterialUI를 전부 공부하기에는 시간이 많지 않아 아직 부족한 부분은 styled-components를 조금씩 쓰기로 하고 프로젝트 마지막 부분에 시간이 난다면 이 부분은 MaterialUI로 수정하는 쪽으로 하면 시간관리 측면에서 더 좋을 것 같습니다!!

FE/src/App.js Outdated
</>
);
};

const MainWrap = styled.div`
Copy link
Contributor

Choose a reason for hiding this comment

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

이 부분은 MaterialUI의 Container를 사용해보는거 어때요?
styled-component를 사용하지 않고 가운데 정렬하는 Container를 구현할 수 있을 거 같아요.

Container : The container centers your content horizontally. It's the most basic layout element.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

와 감사합니다 Container를 몰랐는데 덕분에 알게 되었습니다! 수정하도록 하겠습니다!

FE/src/App.js Outdated
</>
);
};

const MainWrap = styled.div`
margin: 0 auto;
width: 80%;
Copy link
Contributor

Choose a reason for hiding this comment

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

해당 API 문서maxWidth 속성으로 너비를 조절할 수 있을 거 같아요.

@@ -1,7 +1,76 @@
import React from "react";

import MenuList from "../../../common/MenuList";
Copy link
Contributor

Choose a reason for hiding this comment

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

제가 작업하면서 절대경로를 추가했으니 PR 보낼 때 변경사항 알려드리겠습니다!


import Button from "@material-ui/core/Button";
import Popover from "@material-ui/core/Popover";
import PopupState, { bindTrigger, bindPopover } from "material-ui-popup-state";
Copy link
Contributor

Choose a reason for hiding this comment

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

신기하네요. MaterialUI 드롭다운 버튼 라이브러리인가요???

Copy link
Contributor Author

Choose a reason for hiding this comment

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

popover 이 부분을 참고하여 material-ui-popup-state install 후 사용하였습니다.

border: "var(--button-border)",
},
filterText: {
fontSize: "13px",
Copy link
Contributor

Choose a reason for hiding this comment

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

px을 빼고 13 으로만 넘겨줘도 작동하는 거 같아요. (""도 빼고)

};

const Nav = styled.nav`
Copy link
Contributor

Choose a reason for hiding this comment

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

nav 엘리먼트의 경우, <Box component="nav" /> 식으로 작성할 수 있을 거 같아요. styled-component의 as 속성과 비슷한 MaterialUI 속성인듯 합니다.

Overriding Material-UI components
The Box component wraps your component. It creates a new DOM element, a <div> by default that can be changed with the component property. Let's say you want to use a instead:

<Box component="span" m={1}>
  <Button />
</Box>

// 1. title로써 상단에 사용
// 2. menuList로써 hover와 마우스 클릭이 되는 list로 사용
// title로 사용 시 : props로 title을 사용하고 title에 들어갈 text를 넣어주면 됩니다.
// (예) <MenuList title={"Filter lssues"} />
Copy link
Contributor

Choose a reason for hiding this comment

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

친절한 주석 감사합니다~!

Copy link
Contributor

Choose a reason for hiding this comment

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

<MenuList title="Filter Issues" /> 식으로 문자열의 경우 브라켓을 넣어주지 않아도 동작할 거에요

Copy link
Contributor Author

Choose a reason for hiding this comment

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

앗 이건 옮기는 과정에서 실수입니다!

import reset from "styled-reset";

const variables = css`
:root {
--button-border: 1px solid rgba(27, 31, 35, 0.2);
Copy link
Contributor

Choose a reason for hiding this comment

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

저도 이전 프로젝트를 진행했을 때는 이런 방식으로 스타일을 관리했었는데, 이번엔 라이브러리를 사용하는 만큼 스타일 전역변수들을 최대한(되도록이면 아예...) 사용하지 않는 방법으로 해보려고 하는데 엘린님은 어떠세요?

Github 사이트에서 color값을 추출한 뒤에 직접 만든 css variable 변수를 사용하는 대신, 간격이나 컬러 등 여러 css 속성들을 MaterialUI 내부에서 사용하고 있는 변수로 사용하면 스타일링 하는 시간을 더 줄일 수 있을 거 같아요. 이번에 crong님이 요구사항에 적어놓으신 material UI와 같은 라이브러리의 도움을 최대한 많이 받는다 부분이 이런 색상값을 따로 관리하거나 하는 일을 최대한으로 줄여라. 라고 저는 받아들였거든요.

아래 링크는 기본 테마 변수 목록입니다. MaterialUI에서 제공하는 기본 테마에 맞추고, 별도의 커스터마이징을 '최대한 안할수록' 서로가 만든 UI의 디자인 일관성을 최대한 높일 수 있을 거 같다고 생각해요(MaterialUI에서 만들어놓은 디자인이니까).

Github과 비슷하게 만들고 싶으면 아래 링크를 참고해서 커스터마이징할 수도 있을 거 같아요. 근데 개인적으론 똑같이 따라하는 건 중요치 않다고 생각하는 편입니다...

Copy link
Contributor

Choose a reason for hiding this comment

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

여기 MaterialUI의 디자인 시스템이 깔끔하게 전부 정리되어 있네요. 저도 봐야겠습니다

@sungik-choi sungik-choi added this to Review in progress in FE 이슈 관리 프로젝트 Jun 15, 2020
MuseopKim added a commit that referenced this pull request Jun 15, 2020
MuseopKim added a commit that referenced this pull request Jun 15, 2020
MuseopKim added a commit that referenced this pull request Jun 15, 2020
MuseopKim added a commit that referenced this pull request Jun 15, 2020
@Elllin
Copy link
Contributor Author

Elllin commented Jun 16, 2020

리뷰 달아주신 내용 참고해서 수정 후 다시 pr보내겠습니다! 달아주시고 링크가 정말 큰 도움이 되었습니다. 몰랐던 material-ui기능도 알게되었습니다! 친절한 리뷰 감사합니다^^

@Elllin Elllin closed this Jun 16, 2020
FE 이슈 관리 프로젝트 automation moved this from Review in progress to Done Jun 16, 2020
@sungik-choi sungik-choi deleted the fe/feature/filterMenu branch June 16, 2020 08:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🍺 FE 프론트엔드 이슈
Development

Successfully merging this pull request may close these issues.

[FE] 이슈목록 / 필터 버튼 팝업 UI
2 participants