-
-
Notifications
You must be signed in to change notification settings - Fork 250
Make history duration configurable #1074
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is awesome !
res/values-cs/strings.xml
Outdated
@@ -49,7 +49,7 @@ | |||
<string name="pref_switch_input_immediate_summary">Jak bude klávesa pro přepnutí klávesnice reagovat</string> | |||
<string name="pref_vibrate_custom">Vlastní vibrace</string> | |||
<string name="pref_vibrate_duration_title">Síla vibrace</string> | |||
<!-- <string name="pref_number_entry_title">Layout when typing numbers, dates and phone numbers</string> --> | |||
<!-- <string name="pref_number_entry_title">Layout when typing numbers, dates, and phone numbers</string> --> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like to stop using sync_translations
due to problems with Weblate: #1076
Do you mind dropping these changes ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reverted.
@@ -5,8 +5,10 @@ | |||
import android.content.Context; | |||
import android.os.Build.VERSION; | |||
import java.util.ArrayList; | |||
import java.util.Date; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Date
is not used.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed
res/xml/settings.xml
Outdated
@@ -58,4 +58,7 @@ | |||
<juloo.keyboard2.prefs.IntSlideBarPreference android:key="custom_border_radius" android:title="@string/pref_corners_radius_title" android:summary="%s%%" android:defaultValue="0" min="0" max="100" android:dependency="border_config"/> | |||
<juloo.keyboard2.prefs.SlideBarPreference android:key="custom_border_line_width" android:title="@string/pref_border_width_title" android:summary="%sdp" android:defaultValue="0" min="0" max="5" android:dependency="border_config"/> | |||
</PreferenceCategory> | |||
<PreferenceCategory android:title="@string/pref_category_clipboard"> | |||
<juloo.keyboard2.prefs.IntSlideBarPreference android:key="clipboard_history_duration" android:title="@string/pref_clipboard_history_duration" android:summary="@string/pref_clipboard_history_duration_value" android:defaultValue="5" min="1" max="600" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
600 doesn't feel like a round number of minutes and that makes the slider very impressive because the steps are very small. What about 120 (2 hours) ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
600 feels the most round number to me, it's exactly 10 hours. But I see the slider step issue.
I feel like 120 might be a bit small? Depending on the use case, user might want to keep the history longer. I have reduced it to 240.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure this completely solves the problem. The clipboard history can be lost for several reasons and I don't think a precise duration would help.
The history is not persisted and is cleared when the app is stopped by the system, which might happen often. The history has a fixed size limit, which makes the lifespan of previous items shorter if you keep copying text.
I think this could be made clearer with a choice list instead of slider with choices like "at most 5 minutes", "at most 30 minutes", "until the app stops". This also leaves room for more complex settings, for example to clear the history some time after switching to an other app or to persist the clipboard history for some time (which I'm not in favor).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, made the change
# Conflicts: # res/values/arrays.xml
PR adds a setting that makes clipboard history duration configurable, instead of using hardcoded value of 5 minutes