Rework SettingsValuesVC#1360
Conversation
wutschel
force-pushed
the
fix_settings
branch
from
f656a04 to
3924b6e
Compare
wutschel
force-pushed
the
fix_settings
branch
from
3924b6e to
083e373
Compare
wutschel
changed the title
wutschel
force-pushed
the
fix_settings
branch
from
083e373 to
dbc6b04
Compare
wutschel
force-pushed
the
fix_settings
branch
2 times, most recently
from
897d5c9 to
40554e2
Compare
|
Hmm, moving Edit: The exact opposite change was done in 52197ae to fix layout issues as well. Edit 2: Rework this completely now. Before |
wutschel
force-pushed
the
fix_settings
branch
from
40554e2 to
2069fc0
Compare
wutschel
force-pushed
the
fix_settings
branch
from
2069fc0 to
1950f35
Compare
kambala-decapitator
left a comment
kambala-decapitator
left a comment
There was a problem hiding this comment.
I don't think I understand what's that fuss about cell height...
| (CELL_HEIGHT_DEFAULT - LABEL_HEIGHT_DEFAULT) / 2, | ||
| cell.frame.size.width - 2 * PADDING_HORIZONTAL, | ||
| LABEL_HEIGHT_DEFAULT)]; | ||
| UILabel *cellLabel = [UILabel new]; |
There was a problem hiding this comment.
this huge cell setup should be extracted to a cell subclass
There was a problem hiding this comment.
At the end I will have rewritten all of the app's code ... I guess this can be done in a separate PR?
There was a problem hiding this comment.
sure
At the end I will have rewritten all of the app's code
that's the goal ;) also "Ship of Theseus" :)
| // Identify fmt-like format string | ||
| if (format.length && [format rangeOfString:@"{"].location != NSNotFound && [format rangeOfString:@"}"].location != NSNotFound) { | ||
| NSInteger openBraceLoc = [format rangeOfString:@"{"].location; | ||
| NSInteger closeBraceLoc = [format rangeOfString:@"}" options:NSBackwardsSearch].location; |
There was a problem hiding this comment.
your code will break in a case like {...} smth}42 :) but I think we can assume that it won't happen
Map "slider" to SettingTypeSlider. Set "maximum" to 100% for format "percentage", if maximum is not defined or set to 0. Make sure also "step" < 1 is possible to be processed.
Only show ": " in case the second string is neither nil nor "".
- Introduce SettingValueType - Introduce helper method to read settingValueType from item - Introduce helper method to read settingOptions from item - Move setting variable defaults to beginning of initWithFrame
Introduce getStringForSliderItem which can process both float and integer format. Still uses special handling Kodi < 18.0 and >= 18.0.
Identify the app's internal setting type by delivered "type", "format" and "options". Showing navbar title depends on setting type and is handled in layoutCell.
Extract the unit from fmt-like string formats. This improves the user experience, but avoids a full fmt integration.
Setting cells have different height, depending on their content and mode. To calculate this cell height correctly before heightForRowAtIndexPath is called, the whole layout needs to use the final size, which is fixed and represented by cellWidth. To keep this width, do not use autoresizing. For better centralization of the code, change dimensions only in configureCell and create all UI elements only using new.
wutschel
force-pushed
the
fix_settings
branch
from
2950560 to
7d31af7
Compare
|
Squashed and rebased to master. |
2 participants
Description
Closes #981.
This applies several changes to
SettingsValesVCto fix and improve the functionality.Show labels instead of values
In case of multiselect lists the app showed values (e.g. "-600, -300, ..." or "0, 1, 2") instead of the option names (e.g. "-10 mins, -5 mins, ..."). This is now implemented by mapping the values to option names. As further improvement the navigation bar shows the setting name for such "multiselect" settings.
Option names (each image pair: left=old, right=new)
Fix automatic height of labels
SettingsValuesVC's cells have different height, depending on their content and mode. To calculate this cell height correctly beforeheightForRowAtIndexPathis called, the whole layout needs to use the final size, which is fixed and represented bycellWidth. To keep this width, do not use autoresizing. For better centralization of the code, change dimensions only inconfigureCelland create all UI elements only using new.The screenshots show two examples (before/after) where the old implementation assumed lesser width, which caused the app to assume more height would be needed. The two screenshots on the right (before/after) show, that now the same height is used as for 1-lined labels.
Label heights (each image pair: left=old, right=new)
Support "slider" type
This PR fixes displaying and controlling settings of the type "slider", e.g. "Settings > Games > General > Maximum rewind time" or "Settings > Player > Subtitles > Opacity". Sliders which are reported without a maximum value but use "percentage" as format need to have the maximum slider value set explicitly. This also makes custom buttons work which are created from such setting.
Slider type in number and percentage format (each image pair: left=old, right=new)
Support float and integer for "slider"
JSON API provides the "type" for each setting value. In the case of "slider" this is needed to differentiate between "number" (=
float) and "integer" (=int) values as the formatting for the slider label is different. This will also be needed for future support oflibfmt.To implemented this, the new app-internal type
SettingValueType, and the related code to read this from the JSON API response, was added. Formatting the label text is now taking this type into account. All slider related values are now handled asfloatand only cast to the targeted format when updating the slider's label.Slider with "number" (=float) type
Extract units from fmt-like string formats (experimental)
Even though in #1346 a full integration of libfmt is proposed, I feel this is overkill for just adding some units to slider labels. Therefore, a less complex solution is proposed to parse the unit from fmt-like formats inside
getFormatStringand append the unit to the default format. This improves the user experience without a full libfmt integration.Units on all sliders
Units for custom buttons created from sliders (first one was created before this PR
Summary for release notes
Improvement: Show option labels instead of values
Improvement: Support float slider values
Improvement: Show units for slider values and Kodi >= 18.0
Bugfix: Fix automatic height of settings labels
Bugfix: Fix handling of "slider" settings