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

added page View #224

Merged
merged 3 commits into from
Oct 11, 2024
Merged

added page View #224

merged 3 commits into from
Oct 11, 2024

Conversation

zdoryyk
Copy link
Contributor

@zdoryyk zdoryyk commented Oct 4, 2024

What type of PR is this? (check all applicable)

Description

Added almost all features from issue

Featured Covered in this PR

  • Modify NotePreviewCard to accept an index parameter from home screen's Listview.
  • Modify NotesReadOnlyPage to accept indexas route parameter and pass index from NotePreviewCard which will serve as initial index for swipe navigation.
  • Implement a PageView in NotesReadOnlyPage that allows swiping between notes in read-only mode.
  • Ensure that the notes load correctly when swiped left or right.

Related Tickets & Documents

Screenshots, Recordings

Screen.Recording.2024-10-04.at.13.48.20.mov

Tested Feature??

  • In Real Device.
  • In Emulator

@zdoryyk zdoryyk closed this Oct 4, 2024
@zdoryyk zdoryyk reopened this Oct 4, 2024
@zdoryyk
Copy link
Contributor Author

zdoryyk commented Oct 4, 2024

I should fix that state, But I have no idea how to, can someone help me?

@SankethBK
Copy link
Owner

Hi, the issue is because you are adding notesBloc.add(InitializeNote(id: noteId); inside itemBuilder, when this happens a new note is fetched and the new note is sent to all the BlocBuilders listening to notesBloc. So as a result both current and next/previous pages are getting updated with same note.

I have added fix in this commit 290b2d090ad4a7a74d031ba027be70a926aa61b3, just cherry-pick this commit to your PR.

As a fix i have added buildWhen to rebuild only if the id matches the id of the note we got from index. This ensures we rebuild only for 1 page.

               buildWhen: (previousState, currentState) {
                    
                          if (currentState.safe) {
                            // Rebuild only if the note in the bloc matches the note ID of this page
                            return currentState.id == _notePreview.id;
                          }
                          return false; 
                        },

I also made some more changes

  1. Instead of passing noteIds as route parameter, i am fetching it using BlocProvider.of<NotesFetchCubit>(context);.
  2. Instead of passing index in route parameter, i am iterating through the array of notes and checking which id matches the id we got from route param, the advantage of this is we are no longer dependant on id from route param and the solution can also be extended to the case when we open NotesReadOnlyPage from NoteCreatePage.

Currently there is another small bug: When we open NotesReadOnlyPage from NoteCreatePage by clicking on eye button we are not passing note id as a result it assumes id as 0 and goes to first note. We can pass id here also from route param so that it works correctly for this case as well.

@zdoryyk
Copy link
Contributor Author

zdoryyk commented Oct 4, 2024

ty very much for your response, I got it

@zdoryyk
Copy link
Contributor Author

zdoryyk commented Oct 6, 2024

@SankethBK Hi again, we have also problem with TAGS, When i trying to add, I have null error operator, how can I fix it?

@SankethBK
Copy link
Owner

Hi, can you share a screen recording of the issue

@zdoryyk
Copy link
Contributor Author

zdoryyk commented Oct 7, 2024

Hi, can you share a screen recording of the issue

Look tags

Screen.Recording.2024-10-07.at.09.14.22.mov

@zdoryyk
Copy link
Contributor Author

zdoryyk commented Oct 7, 2024

Screen.Recording.2024-10-07.at.09.14.22.mov

also this bug, when i trying to add tag

@SankethBK
Copy link
Owner

Hey, i tried couple of things, i was able to solve the tags issue but ran into some null value issue somewhere. I will try again and update you by tomorrow

@SankethBK SankethBK changed the base branch from master to swipe-navigation October 11, 2024 09:46
Copy link
Owner

@SankethBK SankethBK left a comment

Choose a reason for hiding this comment

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

I'll merge it to master once i fix these issues

@SankethBK SankethBK merged commit 2bd5c96 into SankethBK:swipe-navigation Oct 11, 2024
2 checks passed
@SankethBK
Copy link
Owner

Hey i have added fixes in this PR
Mainly i had to fix these issues

  1. Don't allow swipe navigation if note read page is opened from note create page as the note could've been edited. For this i wam checking if id is null, means the navigation happened from note create page instead of home page in such cases don't render pageview at all.
  2. Tags issue is happening its being fetched from BlocBuilder, since notesBloc holds the state of currently active note, tags of that note will be updated. Fix was to pass tags from NoteReadOnlyPage rather than reading from BlocBuilder.

SankethBK added a commit that referenced this pull request Oct 11, 2024
* added page View, need help with states in PageView Builder (#224)

* added page View, need help with states in PageView Builder

* added changes, also have problem with Tags, bad state

---------

Co-authored-by: Danil Zdoryk <[email protected]>
Co-authored-by: Sanketh B K <[email protected]>

* wip

* fix nullptr issue

* remove print

* fix formatting

* fix format

---------

Co-authored-by: Danil <[email protected]>
Co-authored-by: Danil Zdoryk <[email protected]>
@zdoryyk
Copy link
Contributor Author

zdoryyk commented Oct 11, 2024

Hello, thanks, I will change it

SankethBK added a commit that referenced this pull request Oct 11, 2024
* added page View, need help with states in PageView Builder (#224)

* added page View, need help with states in PageView Builder

* added changes, also have problem with Tags, bad state

---------

Co-authored-by: Danil Zdoryk <[email protected]>
Co-authored-by: Sanketh B K <[email protected]>

* wip

* fix nullptr issue

* remove print

* fix formatting

* fix format

---------

Co-authored-by: Danil <[email protected]>
Co-authored-by: Danil Zdoryk <[email protected]>
@SankethBK
Copy link
Owner

I already added the changes, just letting you know

@zdoryyk zdoryyk changed the title added page View, need help with states in PageView Builder added page View Oct 28, 2024
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