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

Feat : 학사 일정 api 수정 #172

Merged
merged 30 commits into from
Mar 15, 2024
Merged

Feat : 학사 일정 api 수정 #172

merged 30 commits into from
Mar 15, 2024

Conversation

huiwoo-jo
Copy link
Contributor

📮 관련 이슈

✍️ 구현 내용

  • 학사 화면 api를 수정하였습니다.
  • 데이터 구조로 인하여 함수와 데이터 클래스가 복잡해졌습니다.
  • 기존 구조 문제로 하지 못한 리디자인도 같이 하였습니다.
  • 학사 일정 최소 수치는 2024.01.01 ~ 2025.02.28 로 수정하였으나 지속적인 관리를 위하여 데이터로 수정하는 것이 좋을 수도 있을거 같습니다. local date를 사용하는 것이 오히려 부담이 될 수도 있을거 같아 논의가 필요합니다.

📷 구현 영상

https://github.com/TeamDMU/DMU-Android/assets/84004687/9ee27d50-b16a-403c-b712-bea1bd5b2b88 image

✔️ 확인 사항

  • 컨벤션에 맞는 PR 타이틀
  • 관련 이슈 연결
  • PR 관련 정보 연결 (작업자, 라벨, 마일스톤 등)
  • Github Action 통과

@huiwoo-jo huiwoo-jo added 🎨 DESIGN User interface design 🤗 FEATURE Develop this project 🩵희우 labels Feb 24, 2024
@huiwoo-jo huiwoo-jo self-assigned this Feb 24, 2024
Copy link
Collaborator

@m6z1 m6z1 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 19 to 26
inner class ViewHolder(private val binding: ItemScheduleBinding)
: RecyclerView.ViewHolder(binding.root) {
fun bind(item : Schedule) {
binding.schedule = item
}
fun bind(item: ScheduleEntry) {
binding.schedule = item
binding.tvItemScheduleDate.text = if(item.date[0] == item.date[1]) item.date[0] else "${item.date[0]} ~ \n${item.date[1]}"
binding.tvItemScheduleContents.text = item.content
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

inner class 에 관해서 정리된 아티클인데 한 번 읽어보시면 좋을 것 같아요
블로그 링크

stuKoreanMenuAdapter.submitList(it.first)
stuAnotherMenuAdapter.submitList(it.second)
viewModel.menus.observe(viewLifecycleOwner) {
stuKoreanMenuAdapter.submitList(it)
Copy link
Collaborator

Choose a reason for hiding this comment

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

stu가 어떤 것의 약자인가요 ? 최대한 줄여쓰는 것을 지양하면 좋을 것 같습니다!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

student의 약자인데 기존 코드를 그대로 적으면서 리팩토링을 생각 못 했네요
그리고 이 부분은 메뉴 브랜치의 코드가 섞여버린거 같습니다..🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

이 부분은 학식 관련 리펙토링 브랜치를 따로 만들어서 바꾸는게 좋을거 같습니다

app/src/main/res/layout/item_schedule.xml Outdated Show resolved Hide resolved
Copy link
Member

@hoyahozz hoyahozz left a comment

Choose a reason for hiding this comment

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

코멘트 짧게 남겨보았습니다!

@SerializedName("etc_info")
var etcInfo: String,
@SerializedName("menus")
val menu: List<String>
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
val menu: List<String>
val menus: List<String>

가독성을 위해 컬렉션은 복수형태로 이름 지어주시면 좋을 것 같아요~

Copy link
Contributor Author

Choose a reason for hiding this comment

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

메뉴 관련 브랜치를 따로 제작하여 위에 stu 수정과 같이 수정하는게 좋을거 같습니다.

Copy link
Collaborator

@m6z1 m6z1 left a comment

Choose a reason for hiding this comment

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

확인했습니다 고생했어용 머지 고고

@huiwoo-jo huiwoo-jo merged commit 02f4ce1 into develop Mar 15, 2024
1 check passed
@huiwoo-jo huiwoo-jo deleted the feature/api-schedule branch March 15, 2024 11:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🎨 DESIGN User interface design 🤗 FEATURE Develop this project 🩵희우
Projects
None yet
Development

Successfully merging this pull request may close these issues.

학사 일정 api 수정 학사 일정 화면 리디자인
3 participants