search

common-voice / sentence-collector

Tool to collect and review sentences for Common Voice
https://commonvoice.mozilla.org/sentence-collector/
Mozilla Public License 2.0
81 stars 62 forks source link

Improve UX of Language Selector in Profile #473

Closed MichaelKohler closed 1 year ago

MichaelKohler commented 3 years ago

Currently the profile has a standard dropdown to list all languages. This is currently how the dropdown looks on the "Profile" page:

image

The dropdown is built here: https://github.com/common-voice/sentence-collector/blob/0ea738a73a558b5508e7a7d6c58248642930d115/web/src/components/language-selector.tsx

This could be improved with a combobox that allows searching, see https://omnilingo.xyz/ for an example. This then would make it easier to search for a language and users wouldn't need to scroll through the whole dropdown. Preferably this would use an already existing dropdown available through npm, rather than building our own.

One thing to note here is that the same dropdown is also used for the "Add" and "Review" sections. For those it should still be a dropdown with a selection to make it easier to choose. Therefore I think it would be great if the dropdown implementation would support a "list" (like a normal dropdown) and a search for it.

Thanks @ftyers for suggesting this.

venkateshprabhu2 commented 2 years ago

Can I work on this? How do I get started?

MichaelKohler commented 2 years ago

Yes, this is free to be worked on. Here's the approach I would suggest:

You might want to start with the other issue you commented on first though, as this one here might not be a super easy task at first.

If you have any questions and run into issues, feel free to either post here or join our Matrix chat.

venkateshprabhu2 commented 2 years ago

I will work on this during next week.

MichaelKohler commented 2 years ago

@venkateshprabhu2 how is it going? Are you still interested in working on this? Is there any blocker I could help you with?

venkateshprabhu2 commented 2 years ago

@venkateshprabhu2 how is it going? Are you still interested in working on this? Is there any blocker I could help you with?

@MichaelKohler Sorry about the delay. Office work took over. Give me until the end of this week and I will get back on this.

MichaelKohler commented 2 years ago

Absolutely understandable, we are volunteers here and sometimes we don't have time. No need to be sorry about the delay!

venkateshprabhu2 commented 2 years ago

Thanks for understanding, Michael. Running a bit more late. You will have an update by end of this week.

venkateshprabhu2 commented 2 years ago

I came across https://react.semantic-ui.com/modules/dropdown/ as a top result. Is it good to use?

MichaelKohler commented 2 years ago

Thanks for looking into this. Functionality-wise this looks pretty good to me. Is there a way we could us this without the remaining parts of Semantic UI? What would the impact on the final main.js file be? (You can include it and run npm run build to generate that file and then compare against the file generated currently without modifications)

In the end I'd like to avoid including something that would be blowing up the bundle size just for a dropdown, but can't really say if it would be bad without knowing that number (and I don't currently have time to compare it myself).

HarikalarKutusu commented 2 years ago

An alternative suggestion for this: https://www.npmjs.com/package/react-select

Examples: https://react-select.com/home

venkateshprabhu2 commented 2 years ago

Thank you. @MichaelKohler Should I compare both and share the results?

MichaelKohler commented 2 years ago

That might be interesting. I can't say if there will be a big impact at all though, but worth checking :)

MichaelKohler commented 2 years ago

One thing to note here is that the same dropdown is also used for the "Add" and "Review" sections. For those it should still be a dropdown with a selection to make it easier to choose. Therefore I think it would be great if the dropdown implementation would support a "list" (like a normal dropdown) and a search for it.

Looking at both libraries, I think it would be fine to use the same approach for all dropdowns (Language select in profile, as well as the ones for Review, Add and Stats). Important thing here is to make sure that the language gets preselected if there is only one available.

venkateshprabhu2 commented 2 years ago

Sure. I will work on that. Thanks.

venkateshprabhu2 commented 2 years ago

Here's where I am at right now:

Current main.js without installing anything new - 305 KB.

  1. Using semantic-UI: Running into a build issue when I install it and try to import it. Apparently there is a bug https://github.com/Semantic-Org/Semantic-UI-React/issues/4287 and the solution is to use CDN(add css link). Is this alright to do?

  2. react-select: I Installed, imported it in language-selector module and ran npm build but the size of main remains 305kb. Is this expected or am I missing something?

HarikalarKutusu commented 2 years ago

react-select: I Installed, imported it in language-selector module and ran npm build but the size of main remains 305kb. Is this expected or am I missing something?

Tree-shaking? Did you implement it in JSX code or just imported the module?

MichaelKohler commented 2 years ago

Using semantic-UI: Running into a build issue when I install it and try to import it. Apparently there is a bug https://github.com/Semantic-Org/Semantic-UI-React/issues/4287 and the solution is to use CDN(add css link). Is this alright to do?

Thanks for looking into this. In that case I'd say let's go for react-select, as using a CSS link makes it harder to update (somebody needs to remember to change the version in the link, compared to running upgrades on npm packages).

venkateshprabhu2 commented 2 years ago

react-select: I Installed, imported it in language-selector module and ran npm build but the size of main remains 305kb. Is this expected or am I missing something?

Tree-shaking? Did you implement it in JSX code or just imported the module?

I just imported the module. TIL about tree-shaking. Will implement and compare.

@MichaelKohler Sure, will look into react-select and update here.

(Sorry about the delay again. I will speed it up.)

MichaelKohler commented 2 years ago

(Sorry about the delay again. I will speed it up.)

Absolutely no issue :)

venkateshprabhu2 commented 2 years ago

I imported react-select and used it and ran the build. main.js is 382KiB after that. Without react-select, it is 309 KiB currently. Is that alright?

HarikalarKutusu commented 2 years ago

As per https://bundlephobia.com/package/react-select@5.2.2 that is mostly because of the styling-related dependencies as far as I can see...

At the bottom of that page, there are smaller alternatives. I also found this post: https://bestofreactjs.com/repo/based-ghost-react-functional-select-react-select

venkateshprabhu2 commented 2 years ago

Thank you. I tried react-functional-select and it is coming up to 374 KiB. It's not supposed to be that much? Am I missing something? I imported it and added the example component in the code and ran the build.

HarikalarKutusu commented 2 years ago

react-select has many dependencies, but react-functional-select has none. If you also used styled-components, it will add many of course.

Interesting to see such a rather small functionality would add some 21-23% increase.

venkateshprabhu2 commented 2 years ago

So, should I continue on the implementation with react-select? @MichaelKohler Any thoughts?

MichaelKohler commented 2 years ago

Further thought a bit more about this. Generally I would like to keep the Sentence Collector as lean as possible. Given that not everyone around the world has a great internet connection, I think this should be a goal we're thriving for. That being said, increasing the bundle size to 382 KB is okay to me, as we introduce functionality that is helpful.

@venkateshprabhu2 yes, please go ahead with using react-select. I just had a look at language-selector.tsx and I think we can also simplify the code a bit, I don't think we need Options, Option and NullOption all separated out, given that react-select just takes an array of values. I'd be fine with doing this all in one component. I'll leave it up to you what the best approach here is, but feel free to adjust this component as you think is best :) Thanks for the help here!

MichaelKohler commented 1 year ago

Thanks for bringing this up. The Sentence Collector now has moved to https://commonvoice.mozilla.org/write and therefore is now hosted in the main Common Voice repository. That being said, this issue is no longer relevant as that part of the code does not exist anymore.