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 Content Import Progress Bar 🎉 #109

Merged
merged 15 commits into from
Jun 27, 2018
Merged

Conversation

richtabor
Copy link
Owner

As recorded in #55.

@capuderg
Copy link
Collaborator

Hi @richtabor I've looked at the PR and tested it locally. The demo import progress looks good, but I have a question...

Why did you change some JS code for PluginManager in this commit: 2651c6f ?

Some of these JS code changes (those made in PluginManager part) should not be in this PR.

I've also tidied up some JS code you added to the progress bar part in my last commit.

@capuderg
Copy link
Collaborator

I've also tested the plugin install/activation in this branch and it's not working properly (all plugins get installed, even, if you uncheck some). This is because of the JS code that was changed in the above mentioned commit.

@richtabor
Copy link
Owner Author

I'll double-check. I thought I only changed the text span that was incorrectly replacing the plugin's name with the added class (which changes the checkmark icon color).

@capuderg
Copy link
Collaborator

Yes, please do. Let me know, once it's ready, for the second review.

@richtabor
Copy link
Owner Author

@capuderg I reverted the changes that caused the issue in the first place. I'm not sure if I'm missing any changes that were added in-betweenm but it seems to be working as expected now. 👍

Copy link
Collaborator

@capuderg capuderg left a comment

Choose a reason for hiding this comment

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

I've tested it out and it works OK.

All of the JS code should be refactored one day ... :)

@richtabor
Copy link
Owner Author

Agreed - it's def a bit messy currently.. but it works! 😬

@richtabor richtabor merged commit 6914913 into master Jun 27, 2018
@richtabor richtabor deleted the add-content-import-progress-bar branch June 27, 2018 14:45
@contempoinc
Copy link

Whens this going to be merged and pushed to the "dist" folder? Is it ready to go?

@richtabor
Copy link
Owner Author

@contempoinc I merged it already and just updated the dist. I'll push another release here as well. 👍

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