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 "increment" parameter to ProgressBar to define how much increase the value at every input change #54

Closed
Elius94 opened this issue Dec 7, 2022 · 6 comments
Assignees
Labels
enhancement New feature or request

Comments

@Elius94
Copy link
Owner

Elius94 commented Dec 7, 2022

No description provided.

@Elius94 Elius94 self-assigned this Dec 7, 2022
@Elius94 Elius94 added the enhancement New feature or request label Dec 7, 2022
@h-sifat
Copy link
Contributor

h-sifat commented Jan 11, 2023

Sir, sorry to bother you again. Could you kindly elaborate the feature a little bit? What do you mean by at every input change specifically? Is it just an invocation of the setValue(value) method? Do we need to add new incrementValue()/decrementValue() method?

IMHO, despite good documentation, the constructor signature already seems complex enough with multiple optional parameters. Why don't we just use a single object as the argument. For example:

// with a single object parameter
new Progress({
	x: 3,
	y: 23,
	length: 20,
	id: "prog1",
	thickness: 1,
	style: pStyle,
	theme: "htop",
	orientation: "horizontal",
});

// with multiple parameters
new Progress("prog1", 20, 1, 3, 23, pStyle, "htop", "horizontal")

The previous one is much more clean and scalable than the current implementation. But if you don't want to change the API then should I just add the increment parameter at the end as an optional one?

Thank you.

@Elius94
Copy link
Owner Author

Elius94 commented Jan 11, 2023

Yes, the change of every widget constructor with only one props configuration object is in my plans and it should increment the version to a new major release because of the incompatibility that it will cause.
I'm considerin go do it in the next weeks.
For now, you can ad the increment value as a class property with setter and getter methods.
So if you want to change the default increment you have to use theese methods.

@h-sifat
Copy link
Contributor

h-sifat commented Jan 11, 2023

And, just to be clear what function will the increment property serve? For example, if I create a new ProgressBar and set the *increment value to 10. So, calling some kind of incrementValue method would change the progress percentage by 10 and render it to the screen?

@Elius94
Copy link
Owner Author

Elius94 commented Jan 11, 2023

It will be usefull for interactions with the mouse wheel

if (event.name === "MOUSE_WHEEL_DOWN") {
  if (this.value > this.min + 1) {
      this.value -= 1 // this is the hardcoded increment
  } else {
      this.value = this.min
  }
  this.emit("valueChanged", this.value)
  this.update()
}
if (event.name === "MOUSE_WHEEL_UP") {
  if (this.value < this.max - 1) {
      this.value += 1 // this is the hardcoded increment
  } else {
      this.value = this.max
  }
  this.emit("valueChanged", this.value)
  this.update()
}

This also when will be developed the arrows, on the key listeners.
Thanks man

@Elius94
Copy link
Owner Author

Elius94 commented Jan 11, 2023

Thanks to @h-sifat 🙏

@Elius94 Elius94 closed this as completed Jan 11, 2023
@Elius94
Copy link
Owner Author

Elius94 commented Jan 11, 2023

@h-sifat feel Thank you for your help! Feel free to propose changes for other things as well.
I saw that you use nice and correct syntax, if you feel like, you can for example check for flaws or errors in my code and propose stylistic changes.
Thank you, best regards

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants