Skip to content
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

Bugfixes: Settings view controller #487

Merged
merged 2 commits into from
Nov 26, 2021

Conversation

wutschel
Copy link
Collaborator

Description

Closes #485.
Closes #486.

This PR is just fixing two observed topics which are easily recognized by users: Non-usable displayed values in sliders (e.g. "{0:d}") and truncated strings.

The first issue is caused by Kodi server providing different format strings in different versions. Examples (old -> new -> display):
"%i %%" -> "{0:d} %" -> "3 %" (UI Zoom)
"%i.0 dB" -> "{0:d}.0 dB" -> "96.0 dB" (Music Replaygain)
"%i ms" -> "{0:d} ms" -> "750 ms" (Delays)
"%i" -> "{0:d}" -> "5" (Stereoscopic 3D effect)
This PR handles above cases -- tested with Kodi 17.6 and Kodi 19.3.

The second issue is caused by the App just hardcoding the label height for different types of settings. This PR does not rework this behaviour, but adjusts label's height, numberOfLines and font size to show the full text of the longest string found during testing ("Interface -> Skin -> Stereoscopic 3D effect strength").

The SettingsValueViewController in general needs more rework. The label's height and other UI element's positions (e.g. headers, footers, sliders, toggle switches) should be positioned dynamically based on the text field space requirements.

Screenshots (using Kodi 19.3):

Summary for release notes

Bugfix: Show correct units and complete tex in sliders in the Kodi settings menu

@kambala-decapitator
Copy link
Collaborator

for the first issue I strongly suggest to use the same formatting library that Kodi does instead of reinventing the wheel, as format strings can be quite complex. https://github.com/fmtlib/fmt

including prebuilt library is ok, either static or dynamic.

you might want to check how Kodi builds it: https://github.com/xbmc/xbmc/blob/master/tools/depends/target/libfmt/Makefile. To build yourself (in case you don't want to mess with the source/cmake directly), download Kodi source, configure it to build for iOS (but don't make), cd to this directory and make.

probably I'll be able to provide you a prebuilt version.

@kambala-decapitator
Copy link
Collaborator

probably I'll be able to provide you a prebuilt version

no sorry, Kodi builds only arm64, and we need armv7 as well.

FYI Kodi uses v6.1.2

@wutschel
Copy link
Collaborator Author

Ok, I will take look at this for the final solution. I propose a workaround for now: We can ignore the "formatlabel" completely, which will just drop the unit at the end of the slider value -- e.g. "750" instead of "750 ms". This at least makes the setting visible.

@kambala-decapitator
Copy link
Collaborator

Ok, I will take look at this for the final solution. I propose a workaround for now: We can ignore the "formatlabel" completely, which will just drop the unit at the end of the slider value -- e.g. "750" instead of "750 ms". This at least makes the setting visible.

I agree, that'd already be better

@wutschel
Copy link
Collaborator Author

Ok, I will take look at this for the final solution. I propose a workaround for now: We can ignore the "formatlabel" completely, which will just drop the unit at the end of the slider value -- e.g. "750" instead of "750 ms". This at least makes the setting visible.

I agree, that'd already be better

Did this and rebased to master.

@wutschel
Copy link
Collaborator Author

Is there a chance to find out which API or Kodi server version introduced the new format ("{0:d}" types)? With this info I could already add a condition which would still support the full slider text (e.g. "750 ms") for the old format type ("%i ms"), and only apply the number-only workaround for the new format. I only know Kodi 17.6 uses the old and 19.3 uses the new one.

@kambala-decapitator
Copy link
Collaborator

fmt was introduced in v18 (xbmc/xbmc#11039), but it's difficult to say in which version format strings changed. You could check the latest v18 and make conclusion based on that.

@wutschel
Copy link
Collaborator Author

Just tested this with an existing 18.6 installation on another test VM. This already has the new format. So, I will assume the format was introduced with Kodi 18.x.

@wutschel
Copy link
Collaborator Author

Tested with 17.6, 18.6 and 19.3. 17.6 still shows "0 %" or "750 ms". The other versions will only show "0" or "750" in the same case.

Increase number of lines and the related frame height. In addition, call adjustFontSize to scale the font size down, if still not enough space is provided.
Different Kodi versions use different format strings. For now ignore the format string for Kodi version 18 and above to make the value visible again. Move core functionality into a helper method.
@wutschel
Copy link
Collaborator Author

Squashed and rebased to master.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants