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.

Sign up for GitHub

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

Jump to bottom

feature: allow user to disable alerts #280

Closed
RichardLitt opened this issue Mar 4, 2015 · 7 comments
Closed

feature: allow user to disable alerts #280

RichardLitt opened this issue Mar 4, 2015 · 7 comments

Comments

@RichardLitt
Copy link
Contributor

I've got rangy working in a browserified chrome extension. The extension is always on, and every now and then, rangy does not work in the environment. This happens every time I open a new tab from a Google search link, for instance. In such cases, it throws an alert, although I'm not using rangy at that moment and it is just alerting from the Chrome extension.

Is there a way I can require rangy without enabling alerts? I'd rather the user never see them, or just console.log them.

screenshot 2015-03-04 10 25 35
@RichardLitt RichardLitt changed the title feature: enable user to disable alerts feature: allow user to disable alerts Mar 4, 2015
@RichardLitt RichardLitt mentioned this issue Mar 4, 2015
Closed
@timdown
Copy link
Owner

timdown commented Mar 5, 2015

You can disable alerts by setting rangy.config.alertOnFail = false. The failure message will then go to console instead.

@RichardLitt
Copy link
Contributor Author

Thanks. Works.

@RichardLitt RichardLitt reopened this Mar 8, 2015
@RichardLitt
Copy link
Contributor Author

I may have spoken too soon. Even with this:

var rangy = require('rangy')
rangy.config.alertOnFail = false

I still can see alerts. Is there a way we can pass the config directly into the module?

RichardLitt added a commit to RichardLitt/rangy that referenced this issue Mar 8, 2015
@RichardLitt
Added config option for CommonJS syntax
748582f 
This is controversial. Currently, passing a configuration by using
`rangy.config.alertOnFail = false` after `require('rangy')` does not work
in all environments, I believe because it is async and it isn't able to
access the function as it is wrapped in an IIFE. In order to fix this,
when rangy is initialized, the config has to be loaded in immediately.

This isn't possible with the current syntax, as you can't load in parameters.
After a lot of fiddling around, the best I could come up with was this
solution. However, it is a breaking change - all implementations using require
and CommonJS will have to use this syntax:

```js
var rangy = require('rangy')()
```

To set a config, you would do this:

```js
var rangy = require('rangy')({'alertOnFail': true})
```

I couldn't find a way around this. This solution works for me, closes timdown#280.
@RichardLitt RichardLitt mentioned this issue Mar 8, 2015
Closed
@timdown
Copy link
Owner

timdown commented Mar 17, 2015

I've set the default for that option to false.

@timdown timdown closed this as completed Mar 17, 2015
timdown referenced this issue Mar 18, 2015
Change rangy.config.alertOnFail to false (issue 280).
6d8df8d
@RichardLitt
Copy link
Contributor Author

Thanks @timdown. Can you bump the version for npm?

@timdown
Copy link
Owner

timdown commented Mar 19, 2015

I'm planning to do another beta release at the weekend once I've fixed a few bugs. Can you wait until then?

@RichardLitt
Copy link
Contributor Author

Not a problem. Thanks. 

—
Sent from Mailbox

On Wed, Mar 18, 2015 at 5:20 PM, Tim Down notifications@github.com
wrote:

I'm planning to do another beta release at the weekend once I've fixed a few bugs. Can you wait until then?

Reply to this email directly or view it on GitHub:
#280 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Added config option for CommonJS syntax RichardLitt/rangy
2 participants
@timdown @RichardLitt