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

Add inactivity timeout option #37

Merged
merged 10 commits into from
Oct 25, 2016
Merged

Add inactivity timeout option #37

merged 10 commits into from
Oct 25, 2016

Conversation

zachwills
Copy link
Contributor

@zachwills zachwills commented Oct 25, 2016

Resolves #34

Screenshot:
screen shot 2016-10-25 at 11 00 41 am

@blackus3r
Copy link
Owner

Do you have an idea, how to display the tooltips in a better way? The title tag was a small workaround for my option field, but I think it isn't nice.

@zachwills
Copy link
Contributor Author

If you add something with a class of description it will be italicized and look like a description. That is pretty common with option pages like this.

By the way, do you know how to mark the variable pwl as a global? Codacy keeps yelling about it and the various methods I've tried don't seem to work.

@blackus3r
Copy link
Owner

blackus3r commented Oct 25, 2016

Have a look at the line 100

Maybe you could implement it like that, so the code will not crash, if pwl is not defined.
if (typeof pwl == 'undefined') { var pwl.inactivityTimeout = 5; }

I think, codacy will then stop yelling about it.

@blackus3r
Copy link
Owner

I think, we shoul think about the solution with the description class.

@zachwills
Copy link
Contributor Author

I updated the screenshot with the new description descriptions.

Copy link
Owner

@blackus3r blackus3r left a comment

Choose a reason for hiding this comment

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

Looks good to me :)

wp_localize_script( 'post-worktime-logger', 'pwl',
array( 'ajax_url' => admin_url( 'admin-ajax.php' ) )
);
$pwt_options = get_option("post-worktime-logger-options");
Copy link
Owner

Choose a reason for hiding this comment

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

I have already loaded the options, you can use $pwlOptions

@zachwills
Copy link
Contributor Author

@blackus3r updated to use existing options.

@blackus3r blackus3r merged commit da75864 into blackus3r:swimlane/1.3.0 Oct 25, 2016
@blackus3r
Copy link
Owner

Nice work 👍 Thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants