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

Use new sentence design as default #2351

Open
wants to merge 10 commits into
base: dev
Choose a base branch
from
Open

Use new sentence design as default #2351

wants to merge 10 commits into from

Conversation

trang
Copy link
Member

@trang trang commented May 28, 2020

This PR sets the new sentence design as the default one. Along the way, it fixes some of the remaining issues that were reported so far:

The condition to display sentences with the new design is no longer based on the use_new_design setting but on a new setting, disable_new_design.

The option to use the new design is now reading from this new setting. It is enabled by default (similarly to the random sentence on the homepage) and was moved out of the "Experimental" section into the "Main options" because it's not experimental (although it is temporary).

The setting use_new_design was kept so that we can display the announcement message only to those who had not switch to the new design. People who have switched do not need to see this message.

Wall post asking for tests:
https://tatoeba.org/eng/wall/show_message/35389#message_35389

trang added 8 commits May 27, 2020 00:50
Otherwise each time a list form is opened, all the opened lists are binded to the same instance of the list and are therefore being sync'ed instead of acting independently.

Fixes #2280.
The layout needs to be always "row" for guests since the menu only has one item and doesn't expand.

Refs #2326.
We should allow the contributor to edit when the sentence is adopted and remove the possibility to edit it when it's unadopted.

This also fixes #2232.
@AndiPersti
Copy link
Contributor

You should validate the user input for max_visible_translations because by simply modifying the select control with the browser's dev tools it is possible to set it to any value you want:

root@envy:~# echo 'select settings from users where username = "rumpelstilzchen2";' | mysql tatoeba
settings
{"is_public":"","lang":null,"use_most_recent_list":"","collapsible_translations":"","show_transcriptions":"","sentences_per_page":10,"max_visible_translations":"200","users_collections_ratings":"","native_indicator":"","hide_random_sentence":"0","use_new_design":"0","disable_new_design":"0","default_license":"CC BY 2.0 FR","can_switch_license":false,"new_terms_of_use":"1","license_switch_list_id":null,"hide_new_design_announcement_2":false}

@trang
Copy link
Member Author

trang commented May 31, 2020

Thanks @AndiPersti! I added the validation.

@githubshrek
Copy link
Contributor

Hi @trang do you have plans to push this further along any time soon? I'm guessing not, since it has been in the current status for approaching one year.

@trang
Copy link
Member Author

trang commented May 14, 2021

@githubshrek I'm aware this has been on standby for a while now, but I haven't given up on this :)

This merge request is however blocked by another ticket: #2364 (to order translations in a more meaningful way). We need to solve that ticket first because there has been some reluctance from veteran members of Tatoeba in regards of the new design. It is much less compact than the old design and feels, for some, less efficient to use.

I actually have already started to work on #2364 months ago, I just need to find time where I have a bit of peace of mind to continue it. Then once that is live, we'll have to see if users have any other major issue with the UX of the new design, and if not, we can proceed to switch to the new design as default, and eventually, completely getting rid of the old design and fully focus on improving the new one.

@githubshrek
Copy link
Contributor

I think that your trying to overcomplicate the dependencies. The problem expressed with the new design is that it is much less compact than the old design. The solution therefore is simple: make the new design more compact.

When I compare the new design with the old, I see that the new design is a lot more spaced out. There is, for example, on my screen 14 mm between each flag in the new design and only 10 mm between each flag in the old design (i.e. 40% more spaced out).

New design:
image

Old design:
image

The solution should be to fix the new design, not to fix some completely different part of the system. Sure, I understand how solving "Translations are not ordered in a helpful way" would keep things more compact.

However, I think the "new sentence design" is an epic ticket, and "Translations are not ordered in a helpful way" is a totally different epic ticket. They should not be coupled together, because there is really no genuine dependency.

I would encourage you to make some small easy changes to the new design (e.g. make the flags 10mm spaced rather than 14mm) and in order to get it into production, rather than link it to some other large epic, which no doubt will have its own problems.

The aim should be to make the changes smaller (e.g. adding a "Translations" and "Translations of translations" title to the design) and the dependencies less, to try and get small and steady progress.

@jiru
Copy link
Member

jiru commented May 18, 2021

I actually have already started to work on #2364 months ago

@trang Whoops, I also already started working on this months ago. 🤭 I just pushed that branch and created a draft PR #2791.

trang added a commit that referenced this pull request May 22, 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.

None yet

4 participants