Skip to content

feat: add option 'allowConsoleClears' #30

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

Merged
merged 3 commits into from
Jul 31, 2020
Merged

feat: add option 'allowConsoleClears' #30

merged 3 commits into from
Jul 31, 2020

Conversation

darrinmn9
Copy link
Contributor

Which problem does this PR solve?

#28

When developing locally, developers sometimes utilize the console for helpful debug info. vue-axe should respect if developers don't want the console cleared.

No breaking changes since allowConsoleClears defaults to true.

Let me know if you prefer a different name for allowConsoleClears.

Comment on lines 35 to +38
resetLastNotification()
if (options.allowConsoleClears) {
console.clear()
}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wasn't sure exactly what resetLastNotification does. Not sure if it makes more sense for it to be inside the if (options.allowConsoleClears) block or not?

@ktquez
Copy link
Member

ktquez commented Jul 27, 2020

Thanks @darrinmn9

I will test this week to approve.

@darrinmn9
Copy link
Contributor Author

@ktquez i know your busy, just wanted to give you a bump on this ;). Let me know if you have a response to my other comment or if you'd like me to modify the naming/approach in any way.

@ktquez
Copy link
Member

ktquez commented Jul 31, 2020

Hi @darrinmn9, sorry for the delay in answering you.

I actually had time to test it yesterday, I saw that it works, but having a global clear and a clear for updated I think it will be a little confusing.

I think adding a false to the clear method:

beforeDestroy () {
  clear(false, options)
}

With that, we would put in the documentation that the option clearConsoleOnUpdate clears the console with each component update and change of route.


I have been working on a way to avoid the browser console and show errors using a more user friendly component.
I believe that in August I will finish, we will solve this problem.

@darrinmn9
Copy link
Contributor Author

darrinmn9 commented Jul 31, 2020

ok, so your saying lets close this PR and wait till you arrive at a better solution in Aug? I do agree having 2 console related options could be confusing.

@ktquez
Copy link
Member

ktquez commented Jul 31, 2020

Until v3 becomes available, this PR works and can help at the moment anyone who needs this functionality.

@ktquez ktquez merged commit 712ddd2 into vue-a11y:master Jul 31, 2020
@ktquez
Copy link
Member

ktquez commented Jul 31, 2020

@darrinmn9
Thanks for your PR!

@darrinmn9 darrinmn9 deleted the feat/add-allow-console-clear-option branch July 31, 2020 19:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants