Issue 5243 - Clipped text across languages

Issue 5243 - Clipped text across languages

[diegocarloslima, 2017-05-25 18:53:56]

https://codereview.adblockplus.org/29441558/diff/29441559/adblockplussbrowser...
File adblockplussbrowser/res/xml/preferences_main.xml (left):

https://codereview.adblockplus.org/29441558/diff/29441559/adblockplussbrowser...
adblockplussbrowser/res/xml/preferences_main.xml:30: 
Seems that the title is clipping for the PreferenceScreen in the 'Configure your
filer lists' section in Greek. We probably should create a
MultilinePreferenceScreen as well

https://codereview.adblockplus.org/29441558/diff/29441559/adblockplussbrowser...
File
adblockplussbrowser/src/org/adblockplus/sbrowser/contentblocker/ListedSubscriptionsPreferenceCategory.java
(right):

https://codereview.adblockplus.org/29441558/diff/29441559/adblockplussbrowser...
adblockplussbrowser/src/org/adblockplus/sbrowser/contentblocker/ListedSubscriptionsPreferenceCategory.java:35:
import
org.adblockplus.sbrowser.contentblocker.preferences.MultilinePreferenceCategory;
These imports reordering aren't related to the issue, so they shouldn't be
changed here

https://codereview.adblockplus.org/29441558/diff/29441559/adblockplussbrowser...
File
adblockplussbrowser/src/org/adblockplus/sbrowser/contentblocker/UrlInputOpenerPreference.java
(right):

https://codereview.adblockplus.org/29441558/diff/29441559/adblockplussbrowser...
adblockplussbrowser/src/org/adblockplus/sbrowser/contentblocker/UrlInputOpenerPreference.java:134:
}
To avoid duplication of this snippet here and in all Multiline___Preference, we
should extract this to a utility static method, e.g.
PreferenceUtils.setMultilineTitle(View view) which should be placed in the
'org.adblockplus.sbrowser.contentblocker.util' package

https://codereview.adblockplus.org/29441558/diff/29441559/adblockplussbrowser...
File
adblockplussbrowser/src/org/adblockplus/sbrowser/contentblocker/preferences/MultilineCheckBoxPreference.java
(right):

https://codereview.adblockplus.org/29441558/diff/29441559/adblockplussbrowser...
adblockplussbrowser/src/org/adblockplus/sbrowser/contentblocker/preferences/MultilineCheckBoxPreference.java:1:
package org.adblockplus.sbrowser.contentblocker.preferences;
Our Copyright header is missing here

https://codereview.adblockplus.org/29441558/diff/29441559/adblockplussbrowser...
File
adblockplussbrowser/src/org/adblockplus/sbrowser/contentblocker/preferences/MultilineListPreference.java
(right):

https://codereview.adblockplus.org/29441558/diff/29441559/adblockplussbrowser...
adblockplussbrowser/src/org/adblockplus/sbrowser/contentblocker/preferences/MultilineListPreference.java:1:
package org.adblockplus.sbrowser.contentblocker.preferences;
Our Copyright header is missing here

https://codereview.adblockplus.org/29441558/diff/29441559/adblockplussbrowser...
File
adblockplussbrowser/src/org/adblockplus/sbrowser/contentblocker/preferences/MultilinePreferenceCategory.java
(right):

https://codereview.adblockplus.org/29441558/diff/29441559/adblockplussbrowser...
adblockplussbrowser/src/org/adblockplus/sbrowser/contentblocker/preferences/MultilinePreferenceCategory.java:1:
package org.adblockplus.sbrowser.contentblocker.preferences;
Our Copyright header is missing here

[jens, 2017-05-30 12:17:01]

https://codereview.adblockplus.org/29441558/diff/29441559/adblockplussbrowser...
File adblockplussbrowser/res/xml/preferences_main.xml (left):

https://codereview.adblockplus.org/29441558/diff/29441559/adblockplussbrowser...
adblockplussbrowser/res/xml/preferences_main.xml:30: 
On 2017/05/25 18:53:56, diegocarloslima wrote:
> Seems that the title is clipping for the PreferenceScreen in the 'Configure
your
> filer lists' section in Greek. We probably should create a
> MultilinePreferenceScreen as well

Acknowledged.

https://codereview.adblockplus.org/29441558/diff/29441559/adblockplussbrowser...
File
adblockplussbrowser/src/org/adblockplus/sbrowser/contentblocker/ListedSubscriptionsPreferenceCategory.java
(right):

https://codereview.adblockplus.org/29441558/diff/29441559/adblockplussbrowser...
adblockplussbrowser/src/org/adblockplus/sbrowser/contentblocker/ListedSubscriptionsPreferenceCategory.java:35:
import
org.adblockplus.sbrowser.contentblocker.preferences.MultilinePreferenceCategory;
On 2017/05/25 18:53:56, diegocarloslima wrote:
> These imports reordering aren't related to the issue, so they shouldn't be
> changed here

Acknowledged.

https://codereview.adblockplus.org/29441558/diff/29441559/adblockplussbrowser...
File
adblockplussbrowser/src/org/adblockplus/sbrowser/contentblocker/UrlInputOpenerPreference.java
(right):

https://codereview.adblockplus.org/29441558/diff/29441559/adblockplussbrowser...
adblockplussbrowser/src/org/adblockplus/sbrowser/contentblocker/UrlInputOpenerPreference.java:134:
}
On 2017/05/25 18:53:56, diegocarloslima wrote:
> To avoid duplication of this snippet here and in all Multiline___Preference,
we
> should extract this to a utility static method, e.g.
> PreferenceUtils.setMultilineTitle(View view) which should be placed in the
> 'org.adblockplus.sbrowser.contentblocker.util' package

Acknowledged.

https://codereview.adblockplus.org/29441558/diff/29441559/adblockplussbrowser...
File
adblockplussbrowser/src/org/adblockplus/sbrowser/contentblocker/preferences/MultilineCheckBoxPreference.java
(right):

https://codereview.adblockplus.org/29441558/diff/29441559/adblockplussbrowser...
adblockplussbrowser/src/org/adblockplus/sbrowser/contentblocker/preferences/MultilineCheckBoxPreference.java:1:
package org.adblockplus.sbrowser.contentblocker.preferences;
On 2017/05/25 18:53:56, diegocarloslima wrote:
> Our Copyright header is missing here

Acknowledged.

https://codereview.adblockplus.org/29441558/diff/29441559/adblockplussbrowser...
File
adblockplussbrowser/src/org/adblockplus/sbrowser/contentblocker/preferences/MultilineListPreference.java
(right):

https://codereview.adblockplus.org/29441558/diff/29441559/adblockplussbrowser...
adblockplussbrowser/src/org/adblockplus/sbrowser/contentblocker/preferences/MultilineListPreference.java:1:
package org.adblockplus.sbrowser.contentblocker.preferences;
On 2017/05/25 18:53:56, diegocarloslima wrote:
> Our Copyright header is missing here

Acknowledged.

https://codereview.adblockplus.org/29441558/diff/29441559/adblockplussbrowser...
File
adblockplussbrowser/src/org/adblockplus/sbrowser/contentblocker/preferences/MultilinePreferenceCategory.java
(right):

https://codereview.adblockplus.org/29441558/diff/29441559/adblockplussbrowser...
adblockplussbrowser/src/org/adblockplus/sbrowser/contentblocker/preferences/MultilinePreferenceCategory.java:1:
package org.adblockplus.sbrowser.contentblocker.preferences;
On 2017/05/25 18:53:56, diegocarloslima wrote:
> Our Copyright header is missing here

Acknowledged.

[jens, 2017-05-30 12:30:57]

Added copyright header, extracted static method to set multiline property in
onBindView(), added multi line property for subscriptions screen

[diegocarloslima, 2017-05-31 13:33:58]

https://codereview.adblockplus.org/29441558/diff/29451662/adblockplussbrowser...
File adblockplussbrowser/res/layout/multiline_preference_screen.xml (right):

https://codereview.adblockplus.org/29441558/diff/29451662/adblockplussbrowser...
adblockplussbrowser/res/layout/multiline_preference_screen.xml:1: <?xml
version="1.0" encoding="utf-8"?>
Since this is a general purpose preference layout with multiline title enabled,
I would suggest renaming it to 'multiline_preference.xml'

https://codereview.adblockplus.org/29441558/diff/29451662/adblockplussbrowser...
File adblockplussbrowser/res/xml/preferences_main.xml (right):

https://codereview.adblockplus.org/29441558/diff/29451662/adblockplussbrowser...
adblockplussbrowser/res/xml/preferences_main.xml:10: >
the android:title line shouldn't have changed. the closing angle bracket >
should stay in the same line

[jens, 2017-05-31 13:58:57]

https://codereview.adblockplus.org/29441558/diff/29451662/adblockplussbrowser...
File adblockplussbrowser/res/layout/multiline_preference_screen.xml (right):

https://codereview.adblockplus.org/29441558/diff/29451662/adblockplussbrowser...
adblockplussbrowser/res/layout/multiline_preference_screen.xml:1: <?xml
version="1.0" encoding="utf-8"?>
On 2017/05/31 13:33:58, diegocarloslima wrote:
> Since this is a general purpose preference layout with multiline title
enabled,
> I would suggest renaming it to 'multiline_preference.xml'

Acknowledged.

https://codereview.adblockplus.org/29441558/diff/29451662/adblockplussbrowser...
File adblockplussbrowser/res/xml/preferences_main.xml (right):

https://codereview.adblockplus.org/29441558/diff/29451662/adblockplussbrowser...
adblockplussbrowser/res/xml/preferences_main.xml:10: >
On 2017/05/31 13:33:58, diegocarloslima wrote:
> the android:title line shouldn't have changed. the closing angle bracket >
> should stay in the same line

Acknowledged.

[diegocarloslima, 2017-06-01 14:41:39]

On 2017/05/31 13:58:57, jens wrote:
>
https://codereview.adblockplus.org/29441558/diff/29451662/adblockplussbrowser...
> File adblockplussbrowser/res/layout/multiline_preference_screen.xml (right):
> 
>
https://codereview.adblockplus.org/29441558/diff/29451662/adblockplussbrowser...
> adblockplussbrowser/res/layout/multiline_preference_screen.xml:1: <?xml
> version="1.0" encoding="utf-8"?>
> On 2017/05/31 13:33:58, diegocarloslima wrote:
> > Since this is a general purpose preference layout with multiline title
> enabled,
> > I would suggest renaming it to 'multiline_preference.xml'
> 
> Acknowledged.
> 
>
https://codereview.adblockplus.org/29441558/diff/29451662/adblockplussbrowser...
> File adblockplussbrowser/res/xml/preferences_main.xml (right):
> 
>
https://codereview.adblockplus.org/29441558/diff/29451662/adblockplussbrowser...
> adblockplussbrowser/res/xml/preferences_main.xml:10: >
> On 2017/05/31 13:33:58, diegocarloslima wrote:
> > the android:title line shouldn't have changed. the closing angle bracket >
> > should stay in the same line
> 
> Acknowledged.

LGTM