C4illin / ConvertX

💾 Self-hosted online file converter. Supports 800+ formats
GNU Affero General Public License v3.0
229 stars 3 forks source link

Add search bar for formats #113

Closed luis-c465 closed 2 weeks ago

luis-c465 commented 4 weeks ago

Summary

Have been really enjoying using this tool, and wanted to contribute back.

This pull request completes the Add searchable list of formats from the Todos in the readme

Details

Feel free to let me know if it doesn't line up with what you were thinking for the searchable list, so I can improve it :)

Images

image image image image
C4illin commented 3 weeks ago

Sorry for not reviewing this yet will look it through soon. The design looks awesome!

C4illin commented 3 weeks ago

Some minor things:

luis-c465 commented 3 weeks ago

Mobile Image

Made the following changes

C4illin commented 3 weeks ago

Very nice! I am missing an indication of what format I have selected image Here it should in some way show what format I will convert to. Maybe adding a text below the search bar or replacing the search query with the selected value?

luis-c465 commented 3 weeks ago

I am a bit confused, clicking on a format in the popup should already select the format and then replace the search query with the selected value.

https://github.com/user-attachments/assets/795c88c5-89a0-4f64-ad25-cffedd7a525b

C4illin commented 3 weeks ago

Ohh what that does look perfect. Weird that it doesn't work here, tested multiple browsers with both local and docker :(

luis-c465 commented 3 weeks ago

Had a friend try it out and turned out that only fast clicks (under <100ms) would count for clicking on the format button in the popup.

Made a commit, which should fix it by using onmousedown instead of onclick.

Let me know if it works now :)

C4illin commented 2 weeks ago

It doesn't work unfortunately. And don't we want onclick so that it also registers touch events and keyboard navigation?

C4illin commented 2 weeks ago

Wait now it actually works!

C4illin commented 2 weeks ago

And it also works with onclick

luis-c465 commented 2 weeks ago

[...] and don't we want onclick so that it also registers touch events and keyboard navigation?

Testing it on an iPhone and clicking the buttons works as expected.

Unfortunately, the popup as a whole does not support keyboard navigation. Because clicking tab and focusing any other element on the screen closes the popup :(

C4illin commented 2 weeks ago

Testing it on an iPhone and clicking the buttons works as expected.

Then it is all good! I thought it worked the same way as event listeners do

Unfortunately, the popup as a whole does not support keyboard navigation. Because clicking tab and focusing any other element on the screen closes the popup :(

In that case I think you need to .focus the element so that the keyboard nav starts there. But it isn't that important, so we can skip it for now