Apply bookmark screen design #260

mtkw0127 · 2023-07-11T00:01:22Z

Issue

close Apply bookmark screen design #233

Overview (Required)

I developed BookmarkScreen

Links

Screenshot

Before	After

github-actions · 2023-07-11T00:05:26Z

Snapshot diff report

File name	Image
TimetableScreenTest. checkScrollShot_comp are.png
TimetableScreenTest. checkGridShot_compar e.png
TimetableScreenTest. checkFavoriteToggleS hot_compare.png
TimetableScreenTest. checkGridScrollShot_ compare.png
TimetableScreenTest. checkFavoriteToggleS hot_2_compare.png
TimetableScreenTest. checkLaunchShot_comp are.png
KaigiAppTest.checkNa vigateToBookmarkShot _compare.png
io.github.droidkaigi .confsched2023.sessi ons.component_null_D efaultGroup_Bookmark ItemPreview_0_null_c ompare.png
io.github.droidkaigi .confsched2023.sessi ons.component_null_D efaultGroup_Bookmark FiltersPreview_0_nul l_compare.png
io.github.droidkaigi .confsched2023.sessi ons.component_null_D efaultGroup_PreviewT imetableGridItem_0_n ull_compare.png
KaigiAppTest.checkSt artupShot_compare.pn g

github-actions · 2023-07-11T00:07:04Z

Test Results

18 tests 18 ✔️ 59s ⏱️
  4 suites   0 💤
  4 files   0 ❌

Results for commit 7afc39e.

♻️ This comment has been updated with latest results.

takahirom · 2023-07-11T02:08:04Z

...ions/src/main/java/io/github/droidkaigi/confsched2023/sessions/component/TimetableTopArea.kt

@@ -22,12 +25,14 @@ import androidx.compose.ui.unit.dp
 import io.github.droidkaigi.confsched2023.sessions.strings.SessionsStrings.Timetable

 const val TimetableUiTypeChangeButtonTestTag = "TimetableUiTypeChangeButton"
+const val BookMarkIconTestTag = "BookMarkIconTestTag"


For your reference, now that you added the test tag, you can add test to check the screenshot like checkNavigateToContributorShot() in KaigiAppTest 👍

This is a top-level constant, so perhaps a name like "TimetableBookmarkIconTestTag" would be better.

github-actions · 2023-07-12T15:54:29Z

Dependency diff:

dependency diff%0Abranch: main -> feature/apply-bookmark-screen-design, module: app, configuration: releaseRuntimeClasspath

takahirom · 2023-07-13T01:21:50Z

gradle/libs.versions.toml

@@ -50,7 +50,7 @@ composeUiTestManifest = { module = "androidx.compose.ui:ui-test-manifest", versi
 composeHiltNavigtation = { module = "androidx.hilt:hilt-navigation-compose", version.ref = "composeHiltNavigatiaon" }
 composeLintCheck = { module = "com.slack.lint.compose:compose-lint-checks", version = "1.2.0" }
 composeCoil = { module = "io.coil-kt:coil-compose", version = "2.4.0" }
-
+composeConstraintLayout = { module = "androidx.constraintlayout:constraintlayout-compose", version = "1.0.1" }


It is OK in this PR but ConstraintLayout doesn't support multiplatform. So we maybe have to convert it later. 👀

takahirom · 2023-07-13T01:22:30Z

...e/sessions/src/main/java/io/github/droidkaigi/confsched2023/sessions/section/BookMarkList.kt

@@ -0,0 +1,91 @@
+package io.github.droidkaigi.confsched2023.sessions.section


Having this in section is great!

# Conflicts: # feature/sessions/src/main/java/io/github/droidkaigi/confsched2023/sessions/TimetableScreen.kt # feature/sessions/src/main/java/io/github/droidkaigi/confsched2023/sessions/component/TimetableTopArea.kt

github-actions · 2023-07-17T01:03:54Z

Hi @mtkw0127! Codes seem to be unformatted. To resolve this issue, please run ./gradlew spotlessKotlinApply and fix the results of ./gradlew lintDebug.. Thank you for your contribution.

mtkw0127 · 2023-07-17T01:30:22Z

...ions/src/main/java/io/github/droidkaigi/confsched2023/sessions/component/TimetableTopArea.kt

+                    onClick = { onClickTopAreaBookmarkIcon() },
+                ) {
+                    Icon(
+                        imageVector = Icons.Outlined.Bookmark,


I wanted show outlined bookmarkIcon, but it was filled.
I didn't know how to do that. sorry.
Can I add svg file downloaded from Figma into drawable folder?

mtkw0127 · 2023-07-17T01:35:11Z

.../sessions/src/main/java/io/github/droidkaigi/confsched2023/sessions/section/BookmarkSheet.kt

+            modifier = Modifier
+                .size(84.dp)
+                .background(
+                    color = Color(0xFFCEE9DB),


I didn't want to set Color directly, but I couldn't find the definition.
I didn't know I can add the definition in Color.kt🙏
Is that ok?

mtkw0127 · 2023-07-17T01:52:02Z

...sions/src/main/java/io/github/droidkaigi/confsched2023/sessions/component/BookmarkTopArea.kt

+import androidx.compose.ui.unit.lerp
+import androidx.compose.ui.unit.sp
+import io.github.droidkaigi.confsched2023.sessions.strings.SessionsStrings
+


It was my first challenge to develop animation, so it might be not good in terms of performance or something. 🙏

mtkw0127 · 2023-07-17T01:57:15Z

...sions/src/main/java/io/github/droidkaigi/confsched2023/sessions/component/BookmarkTopArea.kt

+                style = titleTextStyle,
+                modifier = Modifier.padding(
+                    start = if (titlePaddingStart >= 0.dp) titlePaddingStart else 0.dp,
+                    top = if (titlePaddingTop >= 0.dp) titlePaddingTop else 0.dp,


I don't know the reason, but if there is not this if sentence, a crash happen.
The error said that padding is negative value, but I didn't intend to do that.

Can you add a FIXME comment for this?

takahirom · 2023-07-17T10:10:04Z

...sions/src/main/java/io/github/droidkaigi/confsched2023/sessions/component/BookmarkFilters.kt

+    val selectedChipColor = AssistChipDefaults.assistChipColors(
+        containerColor = Color(0xFFCEE9DB),
+    )
+    val selectedChipBoarderColor = AssistChipDefaults.assistChipBorder(
+        borderColor = md_theme_light_outline,
+        borderWidth = 0.dp,
+    )
+    val isAll = currentDayFilter.size == DroidKaigi2023Day.values().size
+    val isDayFirst =
+        currentDayFilter.size == 1 && currentDayFilter.first() == DroidKaigi2023Day.Day1
+    val isDaySecond =
+        currentDayFilter.size == 1 && currentDayFilter.first() == DroidKaigi2023Day.Day2
+    val isDayThird =
+        currentDayFilter.size == 1 && currentDayFilter.first() == DroidKaigi2023Day.Day3


Thanks! We are creating and using UiState for each section. So how about migrating these logics to the UiState? Maybe we don't have to pass currentDayFilter 👀
https://github.com/DroidKaigi/conference-app-2023#screen

But it is ok for now I can make an issue for this

...sions/src/main/java/io/github/droidkaigi/confsched2023/sessions/component/BookmarkTopArea.kt

...ions/src/main/java/io/github/droidkaigi/confsched2023/sessions/component/TimetableTopArea.kt

takahirom

Thanks! It's great point to start! We can improve with our contributors.

Move to BookMarkScreen

f56487e

takahirom reviewed Jul 11, 2023

View reviewed changes

mtkw0127 added 5 commits July 11, 2023 23:22

Fix lint error

86d24dd

Fix the name of testTag

c290a0a

Add BookMarkScreenViewModel

7e04ea3

Fix lint error

43886bb

Use sealed interface for UiState

68c1400

DroidKaigi deleted a comment from github-actions bot Jul 11, 2023

Add Empty BookMarkScreen

d275816

Add List BookMarkScreen Roughly

7657705

takahirom reviewed Jul 13, 2023

View reviewed changes

mtkw0127 added 12 commits July 16, 2023 16:30

Merge branch 'main' into feature/apply-bookmark-screen-design

f759753

# Conflicts: # feature/sessions/src/main/java/io/github/droidkaigi/confsched2023/sessions/TimetableScreen.kt # feature/sessions/src/main/java/io/github/droidkaigi/confsched2023/sessions/component/TimetableTopArea.kt

Add Filter Feature

24978d8

Toggle bookmark

b8cc144

Add OnBackPress

e637c41

Show Filter in EmptyView

e2b91c8

Sort BookMarkItems

ae20277

Fix TopBar

b4e44bf

Fix TopBar

f36dedc

Grouped ListItem By Time

5e61dc1

Grouped ListItem By Time

f557aad

Add checkNavigateToBookmarkShot

736fff7

Replace BookMark to Bookmark

b4fe687

mtkw0127 added 2 commits July 17, 2023 10:11

Add Preview and Fix border

135e6c4

Merge branch 'main' into feature/apply-bookmark-screen-design

fd2b163

mtkw0127 added 5 commits July 17, 2023 10:21

Fix ChipBorder

d0b76b2

Fix ChipStyle

341706c

Fix modifier of BookmarkSheet

02986bf

Define const string value in SessionsStrings

3191ac3

Define const string value in SessionsStrings

4b3e70d

mtkw0127 commented Jul 17, 2023

View reviewed changes

mtkw0127 marked this pull request as ready for review July 17, 2023 02:42

mtkw0127 requested a review from takahirom July 17, 2023 02:43

takahirom reviewed Jul 18, 2023

View reviewed changes

...sions/src/main/java/io/github/droidkaigi/confsched2023/sessions/component/BookmarkTopArea.kt Show resolved Hide resolved

Add FIXME

87a42da

takahirom reviewed Jul 18, 2023

View reviewed changes

...ions/src/main/java/io/github/droidkaigi/confsched2023/sessions/component/TimetableTopArea.kt Show resolved Hide resolved

Add FIXME

7afc39e

takahirom approved these changes Jul 18, 2023

View reviewed changes

takahirom merged commit fedfcfd into main Jul 18, 2023
6 checks passed

takahirom deleted the feature/apply-bookmark-screen-design branch July 18, 2023 02:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Apply bookmark screen design #260

Apply bookmark screen design #260

mtkw0127 commented Jul 11, 2023 •

edited

Loading

github-actions bot commented Jul 11, 2023 •

edited

Loading

github-actions bot commented Jul 11, 2023 •

edited

Loading

takahirom Jul 11, 2023

takahirom Jul 11, 2023

github-actions bot commented Jul 12, 2023

takahirom Jul 13, 2023

takahirom Jul 13, 2023

github-actions bot commented Jul 17, 2023

mtkw0127 Jul 17, 2023 •

edited

Loading

mtkw0127 Jul 17, 2023

mtkw0127 Jul 17, 2023

mtkw0127 Jul 17, 2023

takahirom Jul 18, 2023

takahirom Jul 17, 2023

takahirom Jul 18, 2023

takahirom left a comment

		@@ -0,0 +1,91 @@
		package io.github.droidkaigi.confsched2023.sessions.section

Apply bookmark screen design #260

Apply bookmark screen design #260

Conversation

mtkw0127 commented Jul 11, 2023 • edited Loading

Issue

Overview (Required)

Links

Screenshot

github-actions bot commented Jul 11, 2023 • edited Loading

github-actions bot commented Jul 11, 2023 • edited Loading

Test Results

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Jul 12, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Jul 17, 2023

mtkw0127 Jul 17, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

takahirom left a comment

Choose a reason for hiding this comment

mtkw0127 commented Jul 11, 2023 •

edited

Loading

github-actions bot commented Jul 11, 2023 •

edited

Loading

github-actions bot commented Jul 11, 2023 •

edited

Loading

mtkw0127 Jul 17, 2023 •

edited

Loading