Skip to content

Conversation

@khlr
Copy link
Contributor

@khlr khlr commented Apr 16, 2025

This pull request solves issue #97.

While making these changes I realized that I created quite bunch of spaghetti code when I converted the web extension some years ago... 🙈 And I'm afraid by adding a second filter criteria I even added some noodles...

If I had more time, I'd be itching to rewrite the whole thing properly typed with TypeScript... But that's not very realistic due to the said lack of time 😄

Any way... As the PR changes a couple of lines and some logic I'd be glad if someone would find the time to test the changes. In particular, the export and import logic with and without activated filters. Before the release of a new version, I would feel better if it wasn't just me who had done a bit of testing.

BTW, some info what has changed in the UI:
The former button Filter resources is now called Hide resources which is a better term, I think, as it better describes that the filter is exclusive. And there's now the new button Show protocol requests only which is not enabled by default.

image

@khlr khlr requested review from thijskh and tvdijen April 16, 2025 13:28
case 'application/font-woff2':
case 'application/x-font-ttf':
case 'application/x-font-woff':
case 'font/woff2':
Copy link
Member

Choose a reason for hiding this comment

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

This one should already be filtered by the case on line 612/616

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're absolutely right. Can't tell why I thought, this'd be required. I'll revert it.

@tvdijen
Copy link
Member

tvdijen commented Apr 16, 2025

Code looks fine! Haven't tested it yet

@khlr
Copy link
Contributor Author

khlr commented Apr 22, 2025

Thank you for the review, @thijskh
I'll still wait for your testing, @tvdijen ☺️

Copy link
Member

@tvdijen tvdijen left a comment

Choose a reason for hiding this comment

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

Well done @khlr ! Let's merge & release.

@tvdijen tvdijen linked an issue May 7, 2025 that may be closed by this pull request
@khlr khlr merged commit 85c4e99 into main May 15, 2025
@khlr khlr deleted the feature/filter-for-protocol-requests branch May 15, 2025 18:41
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.

Please add a button that filters out any non-SAML requests

4 participants