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

Source filters, move search to SourceMangas #142

Merged
merged 15 commits into from
Mar 4, 2022

Conversation

Robonau
Copy link
Contributor

@Robonau Robonau commented Mar 1, 2022

implementation of the /api/v1/source/{sourceId}/filters endpoint

Screenshot_20220301_222850
right side (mangadex, different per source):

Screenshot_20220301_222926

groups appear on top of the filters:
Screenshot_20220301_223010

something JUI already implemented

much the same as the library version
base + SelectFilter + CheckBoxFilter
haven't done useQueryParam yet
left to do:
1.saving the input data (so it don't disappear when closing the drawer)
2.post to update
3.reload list after drawer close
@Robonau
Copy link
Contributor Author

Robonau commented Mar 1, 2022

should i combine search in to browse
or should i move filtering over to search?
(personally i thing combining search in to browse makes more sense)
i currently have filter coded for the browse page (/popular), but filtering on the /search endpoint

@Robonau
Copy link
Contributor Author

Robonau commented Mar 2, 2022

rather than using resetFilterValue and updateFilterValue i should probably use a useState
and pass through the set

spamming too many options in quick succession will break it, i should be able to fix if i move to useStates

@AriaMoradi
Copy link
Member

your commit naming is fun

worked out the breaking that was happening in the last commit

im probably using useState for more than is nessessary, only really needed to do triggerUpdate and Data
@Robonau
Copy link
Contributor Author

Robonau commented Mar 2, 2022

i never know what to actually call my commits, so they tend to get pretty random

worked out the breaking that was happening in the last commit.
im probably using useState for more than is necessary, only really needed to do triggerUpdate and Data.
but im done for now, imma open for review.

@Robonau Robonau marked this pull request as ready for review March 2, 2022 19:40
@Robonau
Copy link
Contributor Author

Robonau commented Mar 2, 2022

Screenshot_20220302_194156
search is always right under the reset button

Copy link
Member

@AriaMoradi AriaMoradi left a comment

Choose a reason for hiding this comment

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

Here are a brief changes list from visual testing

  • the looks need a change to become inline with rest of the design(steal design from Tachiyomi)
  • move search to Appbar
  • convert the filter button to a FAB (copying Tachiyomi, not sure which design is better?)
  • don't perfom search at filter input changes, have a Apply/Submit button to reduce network calls, optimize filter update network calls
  • when filter results are loading, MangaGrid shows "No Manga Was found", instead it should show a loading spinner

I'll do a code review when at least non-design issues are fixed.

@Robonau
Copy link
Contributor Author

Robonau commented Mar 3, 2022

screenshots update:
Screenshot_20220303_232253
Screenshot_20220303_232841
Screenshot_20220303_232853
Screenshot_20220303_232917

@AriaMoradi AriaMoradi changed the title Source filters Source filters, move search to SourceMangas Mar 3, 2022
@AriaMoradi AriaMoradi merged commit 964186c into Suwayomi:master Mar 4, 2022
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