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

fix focus effect on up-tree #3

Merged
merged 3 commits into from
Aug 13, 2016
Merged

fix focus effect on up-tree #3

merged 3 commits into from
Aug 13, 2016

Conversation

DennyDai
Copy link
Contributor

Hi, I found another problem like the problem before.
the same problem still exists in the up-tree row.

before

after

@@ -235,6 +236,7 @@ var domUtils = {
ns[i].insertAdjacentHTML('afterend', '<td></td>');
}
}
uptree[3].insertAdjacentHTML('afterend', '<td></td>');
Copy link
Owner

Choose a reason for hiding this comment

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

This will throw error if uptree is not defined, which is a valid case

@DennyDai
Copy link
Contributor Author

Changed already, is that work?

@@ -237,6 +238,10 @@ var domUtils = {
}
});
}
if (uptree && uptree[3]) {
Copy link
Owner

@softvar softvar Aug 13, 2016

Choose a reason for hiding this comment

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

Now the issue is:

This code will remove all the elements having download class, right?
But, as per the above written code, which makes an AJAX call and then do something, which will always be called after this piece of code as it is fetching data from server, async task.

So, what will happen is: once data is fetched, the code will internally executes utils.removePrevInstancesOf('.download'); which in turn will remove the newly added td for uptree.

So, this code breaks up.

Solution is:

Add the below code:

if (uptree && uptree[3]) {
   uptree[3].insertAdjacentHTML('afterend', '<td class="download"></td>');
}

after these lines:

...
data = utils.sortFileStructureAsOnSite(data);

        if (!data) { return; }

        utils.removePrevInstancesOf('.download'); // remove before adding new ones

which will handle it.

Also, please check once before pushing. The procedure is simple and is documented in the README.md.

  1. create a branch and then work, don't commit to master directly
  2. run npm install once and then gulp watch
  3. do your changes, update the extension by clicking on refresh button, after loading it as unpacked - chrome://extensions
  4. Verify the changes
  5. Push them, happy coding!

@softvar softvar merged commit 338e1fe into softvar:master Aug 13, 2016
@softvar
Copy link
Owner

softvar commented Aug 13, 2016

Thanks for the patch!

softvar pushed a commit that referenced this pull request Apr 8, 2021
…ni-1.3.8

chore(deps): bump ini from 1.3.5 to 1.3.8
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