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

Dev gui contact #335

Open
wants to merge 1 commit into
base: staging
Choose a base branch
from
Open

Dev gui contact #335

wants to merge 1 commit into from

Conversation

wo-rasp
Copy link
Contributor

@wo-rasp wo-rasp commented Feb 27, 2017

This pull requests adds handling of CONTACT devices to the GUI. It was tested for SC2262 protocol.
A description on how to test it was added to the wiki for SC2262 protocol. The same principle can be used for all other protocols.

@CurlyMoo
Copy link
Contributor

Can you make contact a readonly element?

@wo-rasp
Copy link
Contributor Author

wo-rasp commented Feb 28, 2017 via email

@wo-rasp
Copy link
Contributor Author

wo-rasp commented Feb 28, 2017

Will remove commit d8c2dd0 and c2345a2 first, as they should not be part of this PR.

@wo-rasp
Copy link
Contributor Author

wo-rasp commented Feb 28, 2017

Problem with the disabled buttons is the opacity of the buttons.

@wo-rasp
Copy link
Contributor Author

wo-rasp commented Mar 5, 2017

Done, tested, confirmed by other user. The opacity for disabled devices is an issue of the browser and intended by design. The development branch is the base of this PR.

oTab = $('#all');
}
if('name' in aValues) {
oTab.append($('<li id="'+sDevId+'" class="contact" data-icon="false"><div class="name">'+aValues['name']+'</div><div id="'+sDevId+'_contact" class="contact" data-role="fieldcontain" data-type="horizontal"><fieldset data-role="controlgroup" class="controlgroup" data-type="horizontal" data-mini="true"><input type="radio" name="'+sDevId+'_contact" id="'+sDevId+'_contact_closed" value="closed" /><label for="'+sDevId+'_contact_closed">'+language.closed+'</label><input type="radio" name="'+sDevId+'_contact" id="'+sDevId+'_contact_opened" value="opened" /><label for="'+sDevId+'_contact_opened">'+language.opened+'</label></fieldset></div></li>'));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Too much spaces here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

$('#'+sDevId+'_contact_closed').bind("change", function(event, ui) {
event.stopPropagation();
if('confirm' in aValues && aValues['confirm']) {
if(window.confirm("Are you sure?") == false) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be language independent.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as with switch protocol

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, you found a bug in the switch protocol 😉

}
$('#'+sDevId+'_contact_opened').checkboxradio('disable');
$('#'+sDevId+'_contact_closed').checkboxradio('disable');
$('#'+sDevId+'_contact_opened').fadeTo(0,0.1);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spaces after the komma.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed. opacity is a function of the browser used for showing elements as being disabled.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fadeTo has no impact for example with Firefox,

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand what it does. I just want the programming style to be consistent.

} else {
bSending = true;
if('all' in aValues && aValues['all'] == 1) {
$.get(sHTTPProtocol+'://'+location.host+'/control/control?device='+sDevId+'&state='+this.value+'&values[all]=1');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The URL is wrong, double /control.
Just saw that this is wrong in createScreenElement() too, on line 399. But that's unrelated to this pr.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, nice catch.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@pilino1234
Copy link
Member

Looks good, just the minor change @CurlyMoo pointed out to fix

@CurlyMoo
Copy link
Contributor

CurlyMoo commented Oct 20, 2017

The switch bug requires a new PR.

This PR should only be focused on a new contact element.

@@ -396,7 +498,7 @@ function createScreenElement(sTabId, sDevId, aValues) {
} else {
bSending = true;
if('all' in aValues && aValues['all'] == 1) {
$.get(sHTTPProtocol+'://'+location.host+'/control/control?device='+sDevId+'&state='+this.value+'&values[all]=1');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also make this a seperate PR.

@CurlyMoo CurlyMoo changed the base branch from development to staging July 20, 2018 18:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants