ShowCaseView on Interactive Book #170

aman-singh7 · 2021-11-20T15:40:20Z

Fixes #105

Describe the changes you have made in this PR -
I have added the ShowCaseView package to showcase the button and toggles on the Interactive book and model for storing the state of the button pressed.

Screenshots of the changes (If any) -

Screenrecorder-2021-11-21-17-17-24-212.mp4

Note: Please check Allow edits from maintainers. if you would like us to assist in the PR.

aman-singh7 · 2021-11-21T12:22:57Z

@manjotsidhu Implementation of showcase widget is done from side. I am currently working on the issues of the widget test. Please lemme know if there are any changes required.

manjotsidhu · 2021-11-22T12:33:44Z

Thank you for your contributions @aman-singh7, Initial impressions of the code looks good. I will review and test it very soon.

aman-singh7 · 2021-11-23T08:54:38Z

@manjotsidhu widget test ✔️ .

aman-singh7 · 2021-11-25T06:48:09Z

@manjotsidhu could you give some info to fix the CI failing? 😅

manjotsidhu · 2021-11-25T07:09:30Z

@manjotsidhu could you give some info to fix the CI failing? 😅

The CI is not failing due to your changes.

aman-singh7 · 2021-11-26T10:40:42Z

@manjotsidhu can you now run the workflow? I have rebased the branch with master initially I just merged the master.

coveralls · 2021-11-26T12:57:08Z

Pull Request Test Coverage Report for Build 1610222365

37 of 139 (26.62%) changed or added relevant lines in 7 files are covered.
1 unchanged line in 1 file lost coverage.
Overall coverage decreased (-1.2%) to 72.302%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
lib/utils/router.dart	0	2	0.0%
lib/services/local_storage_service.dart	0	5	0.0%
lib/ui/views/ib/ib_page_view.dart	12	18	66.67%
lib/viewmodels/ib/ib_page_viewmodel.dart	0	12	0.0%
lib/models/ib/ib_showcase.dart	1	19	5.26%
lib/ui/views/ib/ib_landing_view.dart	24	51	47.06%
lib/viewmodels/ib/ib_landing_viewmodel.dart	0	32	0.0%

Files with Coverage Reduction	New Missed Lines	%
lib/ui/views/ib/ib_landing_view.dart	1	66.21%

Totals
Change from base Build 1605446927:	-1.2%
Covered Lines:	3879
Relevant Lines:	5365

💛 - Coveralls

lib/models/ib/ib_showcase.dart

manjotsidhu · 2021-11-26T13:40:18Z

While testing, after showcasing next and previous buttons, it didn't show drawer showcase until I went to next/previous page. Is this intended ?

aman-singh7 · 2021-11-26T13:57:34Z

While testing, after showcasing next and previous buttons, it didn't show drawer showcase until I went to next/previous page. Is this intended ?

The logic to trigger the Drawer Showcase is currently written inside ValueListenableBuilder. So, it will triggered (if not pressed earlier) when there is a content button on that screen

manjotsidhu · 2021-11-26T14:05:22Z

While testing, after showcasing next and previous buttons, it didn't show drawer showcase until I went to next/previous page. Is this intended ?

The logic to trigger the Drawer Showcase is currently written inside ValueListenableBuilder. So, it will triggered (if not pressed earlier) when there is a content button on that screen

I think I opened drawer button earlier so that might be the reason.

manjotsidhu · 2021-11-26T14:07:30Z

I tested again, it triggered drawer showcase an reaching next pages.

manjotsidhu · 2021-11-26T14:14:07Z

Screenrecorder-2021-11-26-19-39-01-11.mp4

aman-singh7 · 2021-11-26T14:16:41Z

I tested again, it triggered drawer showcase an reaching next pages.

I am updating the state of button clicked in onTargetClick. Until and unless button is not pressed in showcaseview it is going to show.

aman-singh7 · 2021-11-26T14:22:29Z

While testing, after showcasing next and previous buttons, it didn't show drawer showcase until I went to next/previous page. Is this intended ?

The logic to trigger the Drawer Showcase is currently written inside ValueListenableBuilder. So, it will triggered (if not pressed earlier) when there is a content button on that screen

*If not pressed implies if not pressed when showcased.

manjotsidhu · 2021-11-27T06:40:36Z

While testing, after showcasing next and previous buttons, it didn't show drawer showcase until I went to next/previous page. Is this intended ?

The logic to trigger the Drawer Showcase is currently written inside ValueListenableBuilder. So, it will triggered (if not pressed earlier) when there is a content button on that screen

*If not pressed implies if not pressed when showcased.

Then the drawer showcase should be independent of toc being there or not. Drawer showcase is important initially.

aman-singh7 · 2021-11-27T07:13:46Z

While testing, after showcasing next and previous buttons, it didn't show drawer showcase until I went to next/previous page. Is this intended ?

The logic to trigger the Drawer Showcase is currently written inside ValueListenableBuilder. So, it will triggered (if not pressed earlier) when there is a content button on that screen

*If not pressed implies if not pressed when showcased.

Then the drawer showcase should be independent of toc being there or not. Drawer showcase is important initially.

Cool.. I will seperate it from toc.

aman-singh7 · 2021-12-03T05:20:02Z

Screenrecorder-2021-12-03-00-03-56-995.mp4

@manjotsidhu I have updated the logic of showcasing. I have a doubt that the current implementation forces the user to click on the button when showcased otherwise it is going to showcase after every page changed. Do we need to force the user or not showcase the button which are previously clicked.

manjotsidhu · 2021-12-03T05:44:13Z

Screenrecorder-2021-12-03-00-03-56-995.mp4

@manjotsidhu I have updated the logic of showcasing. I have a doubt that the current implementation forces the user to click on the button when showcased otherwise it is going to showcase after every page changed. Do we need to force the user or not showcase the button which are previously clicked.

We should not ideally force the user to click the button, if the user dismisses the showcase then it is safe to say that our showcasing is done and no need to show showcase again.

aman-singh7 · 2021-12-08T21:00:19Z

@manjotsidhu can you now test the showcase?

Nitish145 · 2021-12-16T11:19:25Z

@manjotsidhu is the showcase working fine?

manjotsidhu · 2021-12-20T13:48:24Z

Apologies for late reply, thanks for your contribution. I will test the showcase tonight but we will need to fix for iOS as well in order to merge. Would have been better if you have made a new PR for 2.8 since this PR's scope was showcase and we are also including fixes for 2.8.

aman-singh7 · 2021-12-20T20:52:09Z

Apologies for late reply, thanks for your contribution. I will test the showcase tonight but we will need to fix for iOS as well in order to merge. Would have been better if you have made a new PR for 2.8 since this PR's scope was showcase and we are also including fixes for 2.8.

Ok, I will make a separate PR for 2.8

manjotsidhu

✅ Tested and working perfectly on Android
✅ Implementation LGTM 👍

manjotsidhu marked this pull request as ready for review November 21, 2021 12:46

aman-singh7 force-pushed the ib_showcase branch from 2c62d8e to 1a249d4 Compare November 26, 2021 10:37

manjotsidhu reviewed Nov 26, 2021

View reviewed changes

lib/models/ib/ib_showcase.dart Outdated Show resolved Hide resolved

aman-singh7 force-pushed the ib_showcase branch 2 times, most recently from 3b79878 to 31afb98 Compare December 8, 2021 20:15

aman-singh7 requested a review from manjotsidhu December 9, 2021 05:23

aman-singh7 force-pushed the ib_showcase branch from f8b1768 to 1647c87 Compare December 20, 2021 21:00

feat: implement showcase view on interactive book

0b28043

aman-singh7 added 11 commits December 21, 2021 13:27

feat: add test for showcase view

76792bf

feat: wrap drawer with showcase

288b933

rfrac: swap showcase and fab

c3d9472

rfrac: update interactive book test

58be33a

chor: cleanup

c721fe2

rfrac: move showcase logic to respective viewmodel

80eecb9

chor: update contentButton -> tocButton

2741444

bfix,ptch: fix test, rename content button -> toc button

d4c455f

chor: remove whitespace

1aa47b3

bfix: not showcase button mutiple time

e788e4c

rfac: move showcase state logic in viewmodel

53454df

aman-singh7 force-pushed the ib_showcase branch from c5fadd7 to 53454df Compare December 21, 2021 07:59

aman-singh7 added 2 commits December 22, 2021 01:27

bfix: delay showcase

a0dc573

bfix: save the state after every showcase

e01e4e7

manjotsidhu approved these changes Dec 22, 2021

View reviewed changes

manjotsidhu merged commit 0970ada into CircuitVerse:master Dec 22, 2021

aman-singh7 deleted the ib_showcase branch January 12, 2022 17:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ShowCaseView on Interactive Book #170

ShowCaseView on Interactive Book #170

aman-singh7 commented Nov 20, 2021 •

edited

Loading

aman-singh7 commented Nov 21, 2021

manjotsidhu commented Nov 22, 2021

aman-singh7 commented Nov 23, 2021 •

edited

Loading

aman-singh7 commented Nov 25, 2021

manjotsidhu commented Nov 25, 2021

aman-singh7 commented Nov 26, 2021

coveralls commented Nov 26, 2021 •

edited

Loading

manjotsidhu commented Nov 26, 2021

aman-singh7 commented Nov 26, 2021

manjotsidhu commented Nov 26, 2021

manjotsidhu commented Nov 26, 2021

manjotsidhu commented Nov 26, 2021

aman-singh7 commented Nov 26, 2021

aman-singh7 commented Nov 26, 2021 •

edited

Loading

manjotsidhu commented Nov 27, 2021

aman-singh7 commented Nov 27, 2021

aman-singh7 commented Dec 3, 2021

manjotsidhu commented Dec 3, 2021

aman-singh7 commented Dec 8, 2021

Nitish145 commented Dec 16, 2021

manjotsidhu commented Dec 20, 2021

aman-singh7 commented Dec 20, 2021

manjotsidhu left a comment

ShowCaseView on Interactive Book #170

ShowCaseView on Interactive Book #170

Conversation

aman-singh7 commented Nov 20, 2021 • edited Loading

aman-singh7 commented Nov 21, 2021

manjotsidhu commented Nov 22, 2021

aman-singh7 commented Nov 23, 2021 • edited Loading

aman-singh7 commented Nov 25, 2021

manjotsidhu commented Nov 25, 2021

aman-singh7 commented Nov 26, 2021

coveralls commented Nov 26, 2021 • edited Loading

Pull Request Test Coverage Report for Build 1610222365

💛 - Coveralls

manjotsidhu commented Nov 26, 2021

aman-singh7 commented Nov 26, 2021

manjotsidhu commented Nov 26, 2021

manjotsidhu commented Nov 26, 2021

manjotsidhu commented Nov 26, 2021

aman-singh7 commented Nov 26, 2021

aman-singh7 commented Nov 26, 2021 • edited Loading

manjotsidhu commented Nov 27, 2021

aman-singh7 commented Nov 27, 2021

aman-singh7 commented Dec 3, 2021

manjotsidhu commented Dec 3, 2021

aman-singh7 commented Dec 8, 2021

Nitish145 commented Dec 16, 2021

manjotsidhu commented Dec 20, 2021

aman-singh7 commented Dec 20, 2021

manjotsidhu left a comment

Choose a reason for hiding this comment

aman-singh7 commented Nov 20, 2021 •

edited

Loading

aman-singh7 commented Nov 23, 2021 •

edited

Loading

coveralls commented Nov 26, 2021 •

edited

Loading

aman-singh7 commented Nov 26, 2021 •

edited

Loading