Reload BasicUI if sitemap has been changed #3846
Changes from 1 commit
f076d92
69d74a3
3e7aca0
9e98207
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
/** | ||
* Copyright (c) 2014-2017 by the respective copyright holders. | ||
* All rights reserved. This program and the accompanying materials | ||
* are made available under the terms of the Eclipse Public License v1.0 | ||
* which accompanies this distribution, and is available at | ||
* http://www.eclipse.org/legal/epl-v10.html | ||
*/ | ||
package org.eclipse.smarthome.io.rest.sitemap.internal; | ||
|
||
/** | ||
* Event to notify the browser that the sitemap has been changed | ||
* | ||
* @author Stefan Triller - Initial Contribution | ||
* | ||
*/ | ||
public class SitemapChangedEvent extends SitemapEvent { | ||
public final String TYPE = "SITEMAP_CHANGED"; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -515,7 +515,7 @@ public Widget getWidget(Sitemap sitemap, String id) { | |
for (int i = 2; i < id.length(); i += 2) { | ||
w = ((LinkableWidget) w).getChildren().get(Integer.valueOf(id.substring(i, i + 2))); | ||
} | ||
} catch (NumberFormatException e) { | ||
} catch (NumberFormatException | IndexOutOfBoundsException e) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd rather check two lines above if the value is within the bounds. That's cheaper than the control flow by the IOOBE. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Alright, I will change it. Though the |
||
// no valid number, so the requested page id does not exist | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1709,6 +1709,16 @@ | |
value, | ||
title; | ||
|
||
if(data.TYPE === "SITEMAP_CHANGED") { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What happened to your formatter? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nothing :) Grunt expects this format or it complains and does not generate the minified version of the file There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. isn't it also necessary to check if it is this sitemap which changed? I.e. have you tried having to browser windows open with different sitemaps and check that only the affected one gets redirected to its main page? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. While fixing this bug I noticed that its almost impossible to detect in which page the user currently is if the the order of the pages has been changed. Thats why there is a reload for every sitemap change, also for browsers which are on the first page. Just replacing the widgets would have let to inconsistent views in the browser. This solution is now much cleaner. |
||
var oldLocation = window.location.href; | ||
var parts = oldLocation.split("?"); | ||
if(parts.length > 1) { | ||
window.location.href = parts[0] + "?sitemap="+data.sitemapName; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Spaces:
|
||
} else { | ||
window.location.href = oldLocation; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
} | ||
|
||
if (!(data.widgetId in smarthome.dataModel) && (data.widgetId !== smarthome.UI.page)) { | ||
return; | ||
} | ||
|
Large diffs are not rendered by default.
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.
Does it really make sense to create a new event for each callback and not create it once and send it to all of them?
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.
Agreed, creating one event is sufficient.