Skip to content

1단계 buildMethod를 Widget으로 분리 #40

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

Open
wants to merge 14 commits into
base: testrace
Choose a base branch
from

Conversation

testrace
Copy link
Member

안녕하세요 리뷰어님!
시작이 조금 늦은 만큼 열심히 쭉쭉 달려보겠습니다!
많은 피드백 부탁드립니다 🙇

혹시 안드로이드 스튜디오에서 dart 파일을 생성할때 import 'package:flutter/material.dart';
import를 자동(?)으로 적용해주는 플러그인이나 도구 같은게 있을까요?

@Kiboom Kiboom self-requested a review November 17, 2022 13:55
Copy link

@Kiboom Kiboom left a comment

Choose a reason for hiding this comment

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

과제 진행하느라 고생 많으셨습니다! 😀
build 함수를 별도의 위젯으로 잘 분리해주셨습니다 👍

아래 주제와 관련하여 몇 가지 피드백을 드렸는데 확인해주세요! 🚀

• 위젯 상속 없이 위젯 만들기
• 위젯에 필요한 데이터를 외부에서 받아오기
• 변수, 메소드, 위젯을 private으로 만들어보기

),
Spacer(),
Text(
'3,000원',
Copy link

Choose a reason for hiding this comment

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

배달팁 값이 하드코딩 되어있는데요,
4,000원이나 5,000원도 보여줄 수 있도록 유연하게 만들어보면 어떨까요? 🙂

Menu 위젯처럼 프로퍼티로 받아봐도 좋을 듯 해요!

width: 7,
),
Text(
'21,000원 배달 주문하기',
Copy link

Choose a reason for hiding this comment

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

여기도 금액 정보를 유연하게 변경 가능하도록 바꿔봐도 좋을 듯 해요 !


class ButtonBilling extends ElevatedButton {
ButtonBilling({Key? key, required VoidCallback? onPressed})
: super(key: key, onPressed: onPressed, child: children(), style: buttonStyle());
Copy link

Choose a reason for hiding this comment

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

새로운 위젯을 만들 때,
다른 위젯을 상속하기 보다는 레고 부품처럼 활용해보는 걸
먼저 고려해보셔도 좋을 듯 합니다 😀

아래처럼 build() 메소드에서 ElevatedButton을 사용하기만 해도 동일하게 구현할 수 있어요!

class ButtonBilling extends StatelessWidget {
  ButtonBilling({
    Key? key, 
    required VoidCallback? onPressed,
  }) : super(key: key);
  
  @override
  Widget build(BuildContext context) {
    return ElevatedButton(
      child: children(),
      style: buttonStyle(),
      onPressed: onPressed,
    );
  }
}

class Menu extends StatelessWidget {
final String name;
final String description;
final String price;
Copy link

Choose a reason for hiding this comment

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

클래스 내부에서만 사용되는 프로퍼티나 메소드들은
이름 앞에 _를 붙여서 private으로 만드는 것을 권장드려요 !

Copy link
Member Author

@testrace testrace Nov 20, 2022

Choose a reason for hiding this comment

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

final 키워드 앞에 private를 붙일 수가 없더라구요
다트에서는 변수에 _를 붙이면 자동으로 접근제어가 private으로 설정되는 걸까요?
_ prefix로 private을 명시하는 문법은 다트만의 특성인가요?

ClipRRect(
borderRadius: BorderRadius.circular(12),
child: Image.asset(
'images/chickenCartoonImage.jpg',
Copy link

Choose a reason for hiding this comment

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

여기도 이미지 정보를 유연하게 바꿀 수 있도록 바꿔봐도 좋을 듯 해요 !

_buildAddMore(),
_buildBilling(),
ButtonAddMore(),
Billing(),
Copy link

Choose a reason for hiding this comment

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

Billing이나 StoreName, Menu 같은 위젯들은 cart_screen.dart에서만 사용될 것 같아요!

위 위젯들을 cart_screen.dart 파일에서만 사용할 수 있게 하고,
다른 파일에서는 못 쓰게 하려면 어떻게 해야 할까요?

다음 링크를 참고해서 수정해보시겠어요? 🙂

#1 (review)

Copy link
Member Author

Choose a reason for hiding this comment

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

팁 감사합니다!
class에 대한 의존을 제어하는 방식이 굉장히 흥미롭네요 🤩

@Kiboom
Copy link

Kiboom commented Nov 17, 2022

혹시 안드로이드 스튜디오에서 dart 파일을 생성할때 import 'package:flutter/material.dart';
import를 자동(?)으로 적용해주는 플러그인이나 도구 같은게 있을까요?

Flutter 플러그인 설치 후, StatelessWidget이나 StatefulWidget을 입력하면 자동 완성이 뜨는데요,
여기서 import 할 파일들을 자동으로 추천해줍니다 :)

스크린샷 2022-11-18 오전 12 32 35

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants