Skip to content
This repository has been archived by the owner on May 16, 2019. It is now read-only.

switched ssl button to radio #1837

Merged
merged 3 commits into from
Dec 14, 2016
Merged

Conversation

ameliagoodman
Copy link
Contributor

@ameliagoodman ameliagoodman commented Nov 15, 2016

@jjeffryes
Copy link
Contributor

@ameliagoodman can you fix the merge conflicts and update this PR?

@ameliagoodman
Copy link
Contributor Author

@jjeffryes oops. finished that now

Copy link
Contributor

@jjeffryes jjeffryes left a comment

Choose a reason for hiding this comment

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

There's a bug in the render, can you update it?

</div>
<div class="flexCol-5 sslSwitch-radio ustCol-border <% ob.errors.server_ip && print('invalid') %>">
<input type="radio" class="fieldItem" id="js-sslOn" name="sslSwitch" value="true" checked /><label for="js-sslOn" class="radioLabel"><%= polyglot.t('SSLOn') %></label>
<input type="radio" class="fieldItem" id="js-sslOff" name="sslSwitch" value="false" /><label for="js-sslOff" class="radioLabel"><%= polyglot.t('SSLOff') %></label>
Copy link
Contributor

Choose a reason for hiding this comment

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

The state of the radio button isn't being set to match what's in the model on render (that's what <% if(!ob.SSL){ %> hide <% } %> did in the existing code).

You need to add some code around the checked attribute so the input that matches the value in the model is checked, something like:

<% if(ob.SSL) { %>checked<% } %> />

Copy link
Contributor

@jjeffryes jjeffryes left a comment

Choose a reason for hiding this comment

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

One small change I noticed.

</div>
<div class="flexCol-5 sslSwitch-radio ustCol-border <% ob.errors.server_ip && print('invalid') %>">
<% if(ob.SSL) { %>
<input type="radio" class="fieldItem" id="js-sslOn" name="sslSwitch" value="true" checked /><label for="js-sslOn" class="radioLabel"><%= polyglot.t('SSLOn') %></label>
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be better to have one set of radio buttons, and then inside each one, do
` <% if(ob.SSL) { print(checked) } %>
For the true one, and !ob.SSL for the false one.

@jjeffryes jjeffryes merged commit a162fdd into OpenBazaar:master Dec 14, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants