-
-
Notifications
You must be signed in to change notification settings - Fork 312
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
Use sitemap labels instead of name in the sitemap selection dialog #597
Conversation
You would need to save the label of the sitemap in another preference and make the "real" sitemap preference (with the name of the sitemap) hidden. Then, when the user clicks the "Clear sitemap" preference, both preferences need to be cleared. |
@@ -821,13 +821,15 @@ public void onSuccess(Call call, int statusCode, Headers headers, byte[] respons | |||
private void showSitemapSelectionDialog(final List<OpenHABSitemap> sitemapList) { | |||
Log.d(TAG, "Opening sitemap selection dialog"); | |||
final List<String> sitemapNameList = new ArrayList<String>(); | |||
final List<String> sitemapLabelList = new ArrayList<String>(); | |||
for (int i = 0; i < sitemapList.size(); i++) { | |||
sitemapNameList.add(sitemapList.get(i).getName()); |
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.
is it really needed to have two lists here? Wouldn't it be possible to get rid of sitemapNameList? When accessing the sitemapNameList in the dialog, you could access sitemapList, get the element at the position of "item" and execute getName on that.
If we would've API Level 24 as the minimum supported, we could even use streams and wouldn't need any additional list here (at least not as a local variable).
LGTM, however, there're merge conflicts. |
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.
You also need to change the text for the instrumented tests when a sitemap should be selected, right? Currently, the tests are looking for a sitemap with the text "demo", which is now "Main Menu", so they'll fail because demo is not visible to the user.
Fix openhab#595 Signed-off-by: mueller-ma <[email protected]>
Signed-off-by: mueller-ma <[email protected]>
13b8bef
to
fc8d2d5
Compare
Signed-off-by: mueller-ma <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #597 +/- ##
==========================================
- Coverage 31.73% 31.59% -0.15%
==========================================
Files 64 64
Lines 4852 4855 +3
Branches 655 655
==========================================
- Hits 1540 1534 -6
- Misses 3093 3101 +8
- Partials 219 220 +1
Continue to review full report at Codecov.
|
Fix #595
The sitemap name is also shown in the settings. @FlorianSW Whats the best way to show the label there?