Skip to content

Conversation

@guruz
Copy link
Contributor

@guruz guruz commented Jun 15, 2016

Else the checkbox and the decription end up on different lines in my Chrome on OS X

I only saw the settings.js executed, not the PHP. Can anyone tell me where/how the PHP part is used?

@guruz guruz added the design label Jun 15, 2016
@guruz guruz added this to the 9.1-current milestone Jun 15, 2016
@mention-bot
Copy link

By analyzing the blame information on this pull request, we identified @icewind1991, @PVince81 and @Xenopathic to be potential reviewers

@PVince81
Copy link
Contributor

PVince81 commented Jun 15, 2016

@guruz the PHP part might be executed when reloading the page with existing configs.

Ideally in the future all should be rendered in JS only... legacy stuff

Looks good

@PVince81
Copy link
Contributor

Okay, I tried it properly with Amazon S3 and "Enable SSL".

It seems that on master the checkbox value is properly kept on refresh.
But when I switch to your branch, the checkbox isn't ticked any more.

Can you check whether there are other places in the code where the jquery selector might be too specific ?

Else the checkbox and the decription end up on different lines in my Chrome on OS X
@guruz guruz force-pushed the settings_fix_checkbox_newline branch from add931c to 111bd8e Compare June 17, 2016 14:21
@guruz
Copy link
Contributor Author

guruz commented Jun 17, 2016

Should be better now.

I tried improving the alignment a bit more but failed. Seems to be a major PITA https://stackoverflow.com/questions/306252/how-to-align-checkboxes-and-their-labels-consistently-cross-browsers

Maybe something for a followup

@PVince81
Copy link
Contributor

Looks good and checkbox value is kept on refresh 👍

Second review @icewind1991 @Xenopathic @jvillafanez ?

@jvillafanez
Copy link
Member

Looks good 👍

@PVince81 PVince81 merged commit e3ddbeb into master Jun 20, 2016
@PVince81 PVince81 deleted the settings_fix_checkbox_newline branch June 20, 2016 08:00
@individual-it individual-it mentioned this pull request Feb 21, 2017
9 tasks
@individual-it individual-it mentioned this pull request Mar 6, 2017
9 tasks
@lock
Copy link

lock bot commented Aug 5, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Aug 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants