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

Update index.js #1

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

erayhanoglu
Copy link

Hi, there is a small rendering issue in library.
Add some spinners and remove when task completed. you will see that last spinners will persists.
Library ignores render because there is no active spinners. This changes fixes this issue.

Hi, there is a small rendering issue in library. 
Add some spinners and remove when task completed. you will see that last spinners will persists. 
Library ignores render because there is no active spinners. This changes fixes this issue.
@@ -487,8 +487,7 @@ class Spinnies {
currentSpinner.options.status = newStatus;
}
});
this.checkIfActiveSpinners();

Choose a reason for hiding this comment

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

From testing on a fork of the parent "spinnies" project, this, and the other call to this.checkIfActiveSpinners should be left in.

@SweetMNM
Copy link
Owner

SweetMNM commented Apr 13, 2022

Hello @benjamincburns and @erayhanoglu! I am so sorry for ignoring this PR I was not aware of it's existence until now. Maybe I did not understand the problem correctly but I did not manage to replicate it.

This is the code I tried:

const Spinners = require('..');

function wait(ms, cb) {
  setTimeout(cb, ms);
}

const spinners = new Spinners();

spinners.add('first-spinner', { text: 'Lorem Ipsum is simply dummy text', color: 'white' });
spinners.add('second-spinner', { text: 'Lorem Ipsum is very dummy text', color: 'white' });

wait(2500, () => {
  spinners.remove('first-spinner');
})

wait(5000, () => {
  spinners.remove('second-spinner');
  console.log("I should exit now!");
})

It exists fine after logging "I should exit now!", also when exiting no spinners are shown, only the final log (which is the intended behavior since they were removed).

Can you give me some code to replicate your issue? Also node version might be helpful.

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.

None yet

3 participants