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

fixed various issues and updating to beta-2 #1328

Merged
merged 1 commit into from
Jul 31, 2020
Merged

Conversation

fudgepop01
Copy link
Contributor

this will be updating the design and adding features to beta version 2.
Features include two types of filters and an updated look.

@fudgepop01 fudgepop01 requested a review from a team as a code owner July 23, 2020 18:09
Copy link
Contributor

@markwolff markwolff left a comment

Choose a reason for hiding this comment

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

Nice! Did first pass of nit stuff

container.style.left = '0';
container.style.bottom = '0';
container.style.right = '0';
container.style.width = "300px";
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: consistent quotes

@@ -0,0 +1 @@
presentation/
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: nl

if (key.substring(0, 1) === '_') { continue; }
if (CoreUtils.isFunction(ext[key])) {
protoFns.push(key);
for (let i = 0; i < trackers.length; i++) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can for ... of be used here?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Or better use the arrForEach() helper

Comment on lines 182 to 183
console.log(permStyle(prefix));
console.log(permStyle(prefix).substring(1000));
Copy link
Contributor

Choose a reason for hiding this comment

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

remove

Copy link
Collaborator

@MSNev MSNev Jul 29, 2020

Choose a reason for hiding this comment

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

+1 - The console is actually not defined by some browsers unless the dev console is open. So it should be checking whether the console and console.log exist before trying to use. --i.e.create a helper function to do the logging rather than always do the checks (when you want / need to log to the console)

@fudgepop01 fudgepop01 merged commit d584272 into master Jul 31, 2020
@MSNev MSNev added this to the 2.5.7 milestone Aug 3, 2020
@MSNev MSNev deleted the MSDAReba/debugplugin branch August 15, 2020 01:09
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.

3 participants