locale_schema() is varchar(12) so Language code textfield should be the same

CommentFileSizeAuthor
#9 1260752-language_controls.patch585 bytesandypost
#9 locale_no_patch.png31.07 KBandypost
#9 locale_patch.png31.28 KBandypost
#5 1260752-language_controls.patch603 bytesandypost
#2 1260752-language_controls.patch577 bytesandypost
language_controls.patch607 bytesandypost

Comments

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented

1. The database does indeed has a maxlength of 12, so having 60 here is just misleading people. It is going to be truncating the string anyway. So this is a very straightforward and clearly good change.

2. Making the field itself 3 chars longer @andypost explained to me in person that is about making sure that the value is visible even if the font is wider. Which might be workaround for possible browser bugs I think.... I'd assume the field is sized for the wider char to be sure it would fit the data, but this is a browser thing...

andypost’s picture

andypost
he/him
Primary language Russian
commented
StatusFileSize
new 1260752-language_controls.patch577 bytes

I think this was a mistake - size and maxlength should be swapped because all fields on this form has default width (60 from formapi)

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Status: Needs review » Reviewed & tested by the community

This looks very straightforward and simple then as per my #1 comment above.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 1260752-language_controls.patch, failed testing.

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Needs work » Needs review
StatusFileSize
new 1260752-language_controls.patch603 bytes

Re-roll for current d8

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Needs review » Reviewed & tested by the community

back to rtbc

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented

Looks good to me, should be ok to commit.

catch’s picture

catch
he/him
Primary language English
commented
Version: 8.x-dev » 7.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

Thanks! Committed/pushed to 8.x. Will need a re-roll for 7.x.

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Patch (to be ported) » Needs review
StatusFileSize
new locale_patch.png31.28 KB
new locale_no_patch.png31.07 KB
new 1260752-language_controls.patch585 bytes

re-roll for D7

UI before patch

locale_no_patch.png

After patch

locale_patch.png

andypost’s picture

andypost
he/him
Primary language Russian
commented

This is a direct backport but brings a bit of UI change so still needs review...

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Status: Needs review » Reviewed & tested by the community

Looks good, matches form setup to schema so longer language codes are not possible to input.

gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Issue tags: +language-base

Tagging with language base.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
commented
Status: Reviewed & tested by the community » Fixed

It's a little odd to resize the textbox visually at the same time we make its max length different, but this seems like a harmless enough change.

Committed and pushed to 7.x. Thanks!

Automatically closed -- issue fixed for 2 weeks with no activity.