Re: Review Request: Fix bug 234407 - systemsettings iconview wordwrapping cause narrow icon in zhcn locale
On Jan. 9, 2013, 6:43 p.m., Albert Astals Cid wrote: To be honest i don't think this is the correct fix, the correct fix is specifying a minimum space between the items or a minimum size of the items themselves, not forcing the wordwrapping off Thomas Lübking wrote: itemsize (spacing would also increase space between text) and it's actually the only way to fix the bug (look at the cn text of notifications or a11y) and scales better when eg. egypt returned to hieroglyphs or so ;-) Chao Feng wrote: I will try out this method if I have time. If a minimum size is set, the problem of CJK language is indeed solved. But it will change the display of other languages. For this kind of change, all major languages need to checked to prevent another regression. Here is another review request for this idea. See https://git.reviewboard.kde.org/r/108328/ - Chao --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108285/#review25091 --- On Jan. 9, 2013, 4:33 a.m., Chao Feng wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108285/ --- (Updated Jan. 9, 2013, 4:33 a.m.) Review request for kde-workspace. Description --- CJK languages do not use space as words seperator. And a CJK translation of the text in Systemsettings are very short. A single line is enough for them. This addresses bug 234407. http://bugs.kde.org/show_bug.cgi?id=234407 Diffs - systemsettings/icons/IconMode.cpp 37cfc4bed42e4d05fc4c01008f8ca2c63b287b5e Diff: http://git.reviewboard.kde.org/r/108285/diff/ Testing --- 1. Apply patch 2. Systemsetting show ok on CJK Thanks, Chao Feng
Re: Review Request: Fix bug 234407 - systemsettings iconview wordwrapping cause narrow icon in zhcn locale
On Jan. 9, 2013, 10:09 a.m., Ben Cooksley wrote: systemsettings/icons/IconMode.cpp, line 183 http://git.reviewboard.kde.org/r/108285/diff/1/?file=106110#file106110line183 Not sure I like the idea of a hardcoded list of languages... is there a better way of determining if a language is CJK? Christoph Feck wrote: CJK is actually naming the languages which use CJK, so the list is hardcoded by definition. Yichao Yu wrote: I guess the question is not which languages are CJK but which languages have this problem. Yichao Yu wrote: Plus, there may be english text even when the current locale is cjk[1], so I really don't think deciding from the current locale is a good idea. Personally, I like what I am having now (larger spacing?). I don't know what it looks like without word wrap but I think multiple lines is better if the text is really too long (e.g. ?). I did have a suggestion on how the word wrap should be done for cjk (see the last few lines in the description of this bug[2]) i.e. it may be better to keep each lines roughly the same length. For detecting whether word wrapping should be used (and probably what method should be used in order to have a better appearance e.g. using the alternative method for CJK I mentioned above), I think it is probably a better way to detect blank space in the text. It might be a better idea to increase the threshold (maximum length) if there is not a single space in the text and use some better method to do word wrapping in such case. This may work for any language that allow word wrapping (I personally don't know any language that does not, correct me if I am wrong.) (including English (for extremely long words) if you add - correctly). [1] http://wstaw.org/m/2013/01/09/plasma-desktopr20016.png [2] https://bugs.kde.org/show_bug.cgi?id=309780 Xuetian Weng wrote: 1. You are missing zh_HK. 2. Have you ever consider if a non-translated long-english string appear in systemsettings (3rd party, maybe), how would your patch affect the appearance? What I would suggest is set a minimum width for delegate (for all locale), you can use fontMetrics() * constant number, which you can sure the width is longer than maybe 6-8 CJK character, and you will still have good looking for non-CJK gui. I disable wordwrap mode for CJK language because this mode is just a regression for these language. I still clearly remember the day when I upgrade my KDE and open system setting, it looks so ugly :) If it is a regression, than revert it is just a simple way. The old way at least works fine for a long time. It seems most people do not like this simple revert. Here are two possible solution: 1. Set different delegate width: a. If all is non-CJK, no change needed. b. (Not true for most cases) If all items are translated into local CJK language, set width larger c. (Most cases) There are both translated and non-translated items, it is hard to make a decision. For me, I still prefer single line display. But others prefer setting a minimum width, which may make some long English word look bad. Then another solution: 2. Add a configuration option. Let the user choose what they want. - Chao --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108285/#review25038 --- On Jan. 9, 2013, 4:33 a.m., Chao Feng wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108285/ --- (Updated Jan. 9, 2013, 4:33 a.m.) Review request for kde-workspace. Description --- CJK languages do not use space as words seperator. And a CJK translation of the text in Systemsettings are very short. A single line is enough for them. This addresses bug 234407. http://bugs.kde.org/show_bug.cgi?id=234407 Diffs - systemsettings/icons/IconMode.cpp 37cfc4bed42e4d05fc4c01008f8ca2c63b287b5e Diff: http://git.reviewboard.kde.org/r/108285/diff/ Testing --- 1. Apply patch 2. Systemsetting show ok on CJK Thanks, Chao Feng
Re: Review Request: Fix bug 234407 - systemsettings iconview wordwrapping cause narrow icon in zhcn locale
On Jan. 9, 2013, 6:43 p.m., Albert Astals Cid wrote: To be honest i don't think this is the correct fix, the correct fix is specifying a minimum space between the items or a minimum size of the items themselves, not forcing the wordwrapping off Thomas Lübking wrote: itemsize (spacing would also increase space between text) and it's actually the only way to fix the bug (look at the cn text of notifications or a11y) and scales better when eg. egypt returned to hieroglyphs or so ;-) I will try out this method if I have time. If a minimum size is set, the problem of CJK language is indeed solved. But it will change the display of other languages. For this kind of change, all major languages need to checked to prevent another regression. - Chao --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108285/#review25091 --- On Jan. 9, 2013, 4:33 a.m., Chao Feng wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108285/ --- (Updated Jan. 9, 2013, 4:33 a.m.) Review request for kde-workspace. Description --- CJK languages do not use space as words seperator. And a CJK translation of the text in Systemsettings are very short. A single line is enough for them. This addresses bug 234407. http://bugs.kde.org/show_bug.cgi?id=234407 Diffs - systemsettings/icons/IconMode.cpp 37cfc4bed42e4d05fc4c01008f8ca2c63b287b5e Diff: http://git.reviewboard.kde.org/r/108285/diff/ Testing --- 1. Apply patch 2. Systemsetting show ok on CJK Thanks, Chao Feng
Re: Review Request: Fix bug 234407 - systemsettings iconview wordwrapping cause narrow icon in zhcn locale
On Jan. 9, 2013, 10:09 a.m., Ben Cooksley wrote: systemsettings/icons/IconMode.cpp, line 183 http://git.reviewboard.kde.org/r/108285/diff/1/?file=106110#file106110line183 Not sure I like the idea of a hardcoded list of languages... is there a better way of determining if a language is CJK? Christoph Feck wrote: CJK is actually naming the languages which use CJK, so the list is hardcoded by definition. Yichao Yu wrote: I guess the question is not which languages are CJK but which languages have this problem. Yichao Yu wrote: Plus, there may be english text even when the current locale is cjk[1], so I really don't think deciding from the current locale is a good idea. Personally, I like what I am having now (larger spacing?). I don't know what it looks like without word wrap but I think multiple lines is better if the text is really too long (e.g. ?). I did have a suggestion on how the word wrap should be done for cjk (see the last few lines in the description of this bug[2]) i.e. it may be better to keep each lines roughly the same length. For detecting whether word wrapping should be used (and probably what method should be used in order to have a better appearance e.g. using the alternative method for CJK I mentioned above), I think it is probably a better way to detect blank space in the text. It might be a better idea to increase the threshold (maximum length) if there is not a single space in the text and use some better method to do word wrapping in such case. This may work for any language that allow word wrapping (I personally don't know any language that does not, correct me if I am wrong.) (including English (for extremely long words) if you add - correctly). [1] http://wstaw.org/m/2013/01/09/plasma-desktopr20016.png [2] https://bugs.kde.org/show_bug.cgi?id=309780 Xuetian Weng wrote: 1. You are missing zh_HK. 2. Have you ever consider if a non-translated long-english string appear in systemsettings (3rd party, maybe), how would your patch affect the appearance? What I would suggest is set a minimum width for delegate (for all locale), you can use fontMetrics() * constant number, which you can sure the width is longer than maybe 6-8 CJK character, and you will still have good looking for non-CJK gui. Chao Feng wrote: I disable wordwrap mode for CJK language because this mode is just a regression for these language. I still clearly remember the day when I upgrade my KDE and open system setting, it looks so ugly :) If it is a regression, than revert it is just a simple way. The old way at least works fine for a long time. It seems most people do not like this simple revert. Here are two possible solution: 1. Set different delegate width: a. If all is non-CJK, no change needed. b. (Not true for most cases) If all items are translated into local CJK language, set width larger c. (Most cases) There are both translated and non-translated items, it is hard to make a decision. For me, I still prefer single line display. But others prefer setting a minimum width, which may make some long English word look bad. Then another solution: 2. Add a configuration option. Let the user choose what they want. I still clearly remember the day July, 19. in 2009? which may make some long English word look bad. Why? The minimum width is hardly a limit for a long (english) string. Have a look at https://git.reviewboard.kde.org/r/108328/s/1015/ The info icon has quite some glyphs even chinese, ie. CJK does not gurantee short strings - correct? Not using a fixed size grid (but icons of variable width) can be considered a usability bug, for sure it looks dull. As consequence you can have one item with a long line determine the width of all icons and by this cause a pretty spare view. That's hardly a solution either. The problem (assumed from western esthetics) with the chinese glyphs in that screenshot is the wraps for one of them (or maybe two) what could be avoided by relaxed maximum bounds. You'd have to (in the reimplemented ::paint() function of QItemDelegate) match the width of the remaining words against the textrect width, if it's not much more (tm) you'd paint it ignoring clips, otherwise break at the flooring word, move to the next line and go on with the rest of the string. - Thomas --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108285/#review25038 --- On Jan. 9, 2013, 4:33 a.m., Chao Feng wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108285/
Re: Review Request: Fix bug 234407 - systemsettings iconview wordwrapping cause narrow icon in zhcn locale
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108285/#review25038 --- Code itself looks okay, see issue below though. systemsettings/icons/IconMode.cpp http://git.reviewboard.kde.org/r/108285/#comment19173 Not sure I like the idea of a hardcoded list of languages... is there a better way of determining if a language is CJK? - Ben Cooksley On Jan. 9, 2013, 4:33 a.m., Chao Feng wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108285/ --- (Updated Jan. 9, 2013, 4:33 a.m.) Review request for kde-workspace. Description --- CJK languages do not use space as words seperator. And a CJK translation of the text in Systemsettings are very short. A single line is enough for them. This addresses bug 234407. http://bugs.kde.org/show_bug.cgi?id=234407 Diffs - systemsettings/icons/IconMode.cpp 37cfc4bed42e4d05fc4c01008f8ca2c63b287b5e Diff: http://git.reviewboard.kde.org/r/108285/diff/ Testing --- 1. Apply patch 2. Systemsetting show ok on CJK Thanks, Chao Feng
Re: Review Request: Fix bug 234407 - systemsettings iconview wordwrapping cause narrow icon in zhcn locale
On Jan. 9, 2013, 10:09 a.m., Ben Cooksley wrote: systemsettings/icons/IconMode.cpp, line 183 http://git.reviewboard.kde.org/r/108285/diff/1/?file=106110#file106110line183 Not sure I like the idea of a hardcoded list of languages... is there a better way of determining if a language is CJK? CJK is actually naming the languages which use CJK, so the list is hardcoded by definition. - Christoph --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108285/#review25038 --- On Jan. 9, 2013, 4:33 a.m., Chao Feng wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108285/ --- (Updated Jan. 9, 2013, 4:33 a.m.) Review request for kde-workspace. Description --- CJK languages do not use space as words seperator. And a CJK translation of the text in Systemsettings are very short. A single line is enough for them. This addresses bug 234407. http://bugs.kde.org/show_bug.cgi?id=234407 Diffs - systemsettings/icons/IconMode.cpp 37cfc4bed42e4d05fc4c01008f8ca2c63b287b5e Diff: http://git.reviewboard.kde.org/r/108285/diff/ Testing --- 1. Apply patch 2. Systemsetting show ok on CJK Thanks, Chao Feng
Re: Review Request: Fix bug 234407 - systemsettings iconview wordwrapping cause narrow icon in zhcn locale
--- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108285/#review25091 --- To be honest i don't think this is the correct fix, the correct fix is specifying a minimum space between the items or a minimum size of the items themselves, not forcing the wordwrapping off - Albert Astals Cid On Jan. 9, 2013, 4:33 a.m., Chao Feng wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108285/ --- (Updated Jan. 9, 2013, 4:33 a.m.) Review request for kde-workspace. Description --- CJK languages do not use space as words seperator. And a CJK translation of the text in Systemsettings are very short. A single line is enough for them. This addresses bug 234407. http://bugs.kde.org/show_bug.cgi?id=234407 Diffs - systemsettings/icons/IconMode.cpp 37cfc4bed42e4d05fc4c01008f8ca2c63b287b5e Diff: http://git.reviewboard.kde.org/r/108285/diff/ Testing --- 1. Apply patch 2. Systemsetting show ok on CJK Thanks, Chao Feng
Re: Review Request: Fix bug 234407 - systemsettings iconview wordwrapping cause narrow icon in zhcn locale
On Jan. 9, 2013, 6:43 p.m., Albert Astals Cid wrote: To be honest i don't think this is the correct fix, the correct fix is specifying a minimum space between the items or a minimum size of the items themselves, not forcing the wordwrapping off itemsize (spacing would also increase space between text) and it's actually the only way to fix the bug (look at the cn text of notifications or a11y) and scales better when eg. egypt returned to hieroglyphs or so ;-) - Thomas --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108285/#review25091 --- On Jan. 9, 2013, 4:33 a.m., Chao Feng wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108285/ --- (Updated Jan. 9, 2013, 4:33 a.m.) Review request for kde-workspace. Description --- CJK languages do not use space as words seperator. And a CJK translation of the text in Systemsettings are very short. A single line is enough for them. This addresses bug 234407. http://bugs.kde.org/show_bug.cgi?id=234407 Diffs - systemsettings/icons/IconMode.cpp 37cfc4bed42e4d05fc4c01008f8ca2c63b287b5e Diff: http://git.reviewboard.kde.org/r/108285/diff/ Testing --- 1. Apply patch 2. Systemsetting show ok on CJK Thanks, Chao Feng
Re: Review Request: Fix bug 234407 - systemsettings iconview wordwrapping cause narrow icon in zhcn locale
On Jan. 9, 2013, 10:09 a.m., Ben Cooksley wrote: systemsettings/icons/IconMode.cpp, line 183 http://git.reviewboard.kde.org/r/108285/diff/1/?file=106110#file106110line183 Not sure I like the idea of a hardcoded list of languages... is there a better way of determining if a language is CJK? Christoph Feck wrote: CJK is actually naming the languages which use CJK, so the list is hardcoded by definition. I guess the question is not which languages are CJK but which languages have this problem. - Yichao --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108285/#review25038 --- On Jan. 9, 2013, 4:33 a.m., Chao Feng wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108285/ --- (Updated Jan. 9, 2013, 4:33 a.m.) Review request for kde-workspace. Description --- CJK languages do not use space as words seperator. And a CJK translation of the text in Systemsettings are very short. A single line is enough for them. This addresses bug 234407. http://bugs.kde.org/show_bug.cgi?id=234407 Diffs - systemsettings/icons/IconMode.cpp 37cfc4bed42e4d05fc4c01008f8ca2c63b287b5e Diff: http://git.reviewboard.kde.org/r/108285/diff/ Testing --- 1. Apply patch 2. Systemsetting show ok on CJK Thanks, Chao Feng
Re: Review Request: Fix bug 234407 - systemsettings iconview wordwrapping cause narrow icon in zhcn locale
On Jan. 9, 2013, 10:09 a.m., Ben Cooksley wrote: systemsettings/icons/IconMode.cpp, line 183 http://git.reviewboard.kde.org/r/108285/diff/1/?file=106110#file106110line183 Not sure I like the idea of a hardcoded list of languages... is there a better way of determining if a language is CJK? Christoph Feck wrote: CJK is actually naming the languages which use CJK, so the list is hardcoded by definition. Yichao Yu wrote: I guess the question is not which languages are CJK but which languages have this problem. Plus, there may be english text even when the current locale is cjk[1], so I really don't think deciding from the current locale is a good idea. Personally, I like what I am having now (larger spacing?). I don't know what it looks like without word wrap but I think multiple lines is better if the text is really too long (e.g. ?). I did have a suggestion on how the word wrap should be done for cjk (see the last few lines in the description of this bug[2]) i.e. it may be better to keep each lines roughly the same length. For detecting whether word wrapping should be used (and probably what method should be used in order to have a better appearance e.g. using the alternative method for CJK I mentioned above), I think it is probably a better way to detect blank space in the text. It might be a better idea to increase the threshold (maximum length) if there is not a single space in the text and use some better method to do word wrapping in such case. This may work for any language that allow word wrapping (I personally don't know any language that does not, correct me if I am wrong.) (including English (for extremely long words) if you add - correctly). [1] http://wstaw.org/m/2013/01/09/plasma-desktopr20016.png [2] https://bugs.kde.org/show_bug.cgi?id=309780 - Yichao --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108285/#review25038 --- On Jan. 9, 2013, 4:33 a.m., Chao Feng wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108285/ --- (Updated Jan. 9, 2013, 4:33 a.m.) Review request for kde-workspace. Description --- CJK languages do not use space as words seperator. And a CJK translation of the text in Systemsettings are very short. A single line is enough for them. This addresses bug 234407. http://bugs.kde.org/show_bug.cgi?id=234407 Diffs - systemsettings/icons/IconMode.cpp 37cfc4bed42e4d05fc4c01008f8ca2c63b287b5e Diff: http://git.reviewboard.kde.org/r/108285/diff/ Testing --- 1. Apply patch 2. Systemsetting show ok on CJK Thanks, Chao Feng
Re: Review Request: Fix bug 234407 - systemsettings iconview wordwrapping cause narrow icon in zhcn locale
On Jan. 9, 2013, 10:09 a.m., Ben Cooksley wrote: systemsettings/icons/IconMode.cpp, line 183 http://git.reviewboard.kde.org/r/108285/diff/1/?file=106110#file106110line183 Not sure I like the idea of a hardcoded list of languages... is there a better way of determining if a language is CJK? Christoph Feck wrote: CJK is actually naming the languages which use CJK, so the list is hardcoded by definition. Yichao Yu wrote: I guess the question is not which languages are CJK but which languages have this problem. Yichao Yu wrote: Plus, there may be english text even when the current locale is cjk[1], so I really don't think deciding from the current locale is a good idea. Personally, I like what I am having now (larger spacing?). I don't know what it looks like without word wrap but I think multiple lines is better if the text is really too long (e.g. ?). I did have a suggestion on how the word wrap should be done for cjk (see the last few lines in the description of this bug[2]) i.e. it may be better to keep each lines roughly the same length. For detecting whether word wrapping should be used (and probably what method should be used in order to have a better appearance e.g. using the alternative method for CJK I mentioned above), I think it is probably a better way to detect blank space in the text. It might be a better idea to increase the threshold (maximum length) if there is not a single space in the text and use some better method to do word wrapping in such case. This may work for any language that allow word wrapping (I personally don't know any language that does not, correct me if I am wrong.) (including English (for extremely long words) if you add - correctly). [1] http://wstaw.org/m/2013/01/09/plasma-desktopr20016.png [2] https://bugs.kde.org/show_bug.cgi?id=309780 1. You are missing zh_HK. 2. Have you ever consider if a non-translated long-english string appear in systemsettings (3rd party, maybe), how would your patch affect the appearance? What I would suggest is set a minimum width for delegate (for all locale), you can use fontMetrics() * constant number, which you can sure the width is longer than maybe 6-8 CJK character, and you will still have good looking for non-CJK gui. - Xuetian --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108285/#review25038 --- On Jan. 9, 2013, 4:33 a.m., Chao Feng wrote: --- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/108285/ --- (Updated Jan. 9, 2013, 4:33 a.m.) Review request for kde-workspace. Description --- CJK languages do not use space as words seperator. And a CJK translation of the text in Systemsettings are very short. A single line is enough for them. This addresses bug 234407. http://bugs.kde.org/show_bug.cgi?id=234407 Diffs - systemsettings/icons/IconMode.cpp 37cfc4bed42e4d05fc4c01008f8ca2c63b287b5e Diff: http://git.reviewboard.kde.org/r/108285/diff/ Testing --- 1. Apply patch 2. Systemsetting show ok on CJK Thanks, Chao Feng
