search

vtstats / web

VTubers Statistics and Data Visualization
https://vt.poi.cat
134 stars 28 forks source link

PR Segment 3: Channel filter REDO #864

Closed LiquidRekto closed 6 months ago

LiquidRekto commented 6 months ago

Created to match the current branch state of the original repo Presented changes for the channel filter popup

PoiScript commented 6 months ago

again, please create separate pull requests for i18n changes. mixing too much things in one single pull request just making it hard to review, and can lead to merge conflicts.

If some changes is not yet ready, like the compare page, consider creating a draft pull request.

I noticed that you want to replace cdk overlay with a material menu, which is a good idea. But I still encountered some wired behavior in my browser. Perhaps we should focus on implementing the search filter for this pull request, and then address the overlay issue in a separate pull request.

additional notes, always format your code with prettier before commit, you can configure your editor to format on save.

LiquidRekto commented 6 months ago

mixing too much things in one single pull request just making it hard to review

yeah I will take that seriously next time. The last commits long ago I've actually done all of them in haste, so basically, things just keep building up and up, leading to the recent difficulties. Lucky enough this time, the localization part for this section actually comes at the very last, so it's actually easier to handle now.

Perhaps we should focus on implementing the search filter

Regarding of implementing the search filter, do you want me to implement it into the already existing ng-overlay, or still with the mat-menu ?

And, probably I have to create a different PR, since the commits in that branch had a different author name (basically, me but, different).

PoiScript commented 6 months ago

Regarding of implementing the search filter, do you want me to implement it into the already existing ng-overlay, or still with the mat-menu ?

maybe with the existing mat-menu, ideally, pull request should contains minimum code changes.

LiquidRekto commented 6 months ago

Updated to a new PR, please refer to here: https://github.com/vtstats/web/pull/865