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 folder "does not have" filter #767

Merged
merged 3 commits into from
Oct 31, 2022
Merged

added folder "does not have" filter #767

merged 3 commits into from
Oct 31, 2022

Conversation

PiantaSE
Copy link
Contributor

@PiantaSE PiantaSE commented Sep 20, 2022

Hey Boris! This was my attempt at adding the filter stated in the Issue. I feel as I have a solid foundation with it so far, but seem to be missing something, and I cannot put my finger on it. I somehow also have made it so the filter for excluding files with based on the inputted word works opposite of its intended functionality. Thank you for all your help! Look forward to working on this more :)

@whyboris
Copy link
Owner

Sorry it's been a busy month - I somehow missed this PR 🙇
Thank you for the contribution! It looks good as far as I can tell -- I'll try to find time this weekend to test locally and merge 🤞

@whyboris
Copy link
Owner

Thank you so much @PiantaSE for your contribution 🙇

I'm sorry I'm unsure how we ended up with two PRs resolving the same issue 😓 I should have been more vigilant.

I'll merge #772 first, but I see you've caught a few other places where code needed to be updated (e.g. this.onEnterKey(filter, 8) - so I'll try to merge your changes after 🤞

Copy link
Owner

@whyboris whyboris left a comment

Choose a reason for hiding this comment

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

Thank you for catching the tricky stuff in the home.component.ts -- I would have forgotten about those 😅

@whyboris
Copy link
Owner

Thank you for catching all the extra edge cases 🙇

@whyboris whyboris merged commit adf646e into whyboris:main Oct 31, 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