DenverCoder1 / github-readme-streak-stats

🔥 Stay motivated and show off your contribution streak! 🌟 Display your total contributions, current streak, and longest streak on your GitHub profile README
https://streak-stats.demolab.com
MIT License
4.82k stars 750 forks source link

🐛 Demo site: It is sometimes possible to add a color property twice #570

Closed DenverCoder1 closed 1 year ago

DenverCoder1 commented 1 year ago

Describe the bug Sometimes it is possible to add a color property twice (eg. two "ExcludingDaysLabel" colors. This shouldn't be allowed.

To Reproduce Steps to reproduce the behavior:

  1. Visit the demo site - https://streak-stats.demolab.com/demo/
  2. Expand "Advanced Options"
  3. Click the add property (+) button 11 times (until all properties are added)
  4. Click the remove (−) button next to the "Dates" property
  5. Click the add property (+) button while "ExcludingDaysLabel" is selected in the menu
  6. "ExcludingDaysLabel" gets added again even though it's already added

Expected behavior Properties already added should not appear in the dropdown.

Screenshots If applicable, add screenshots to help explain your problem.

image

image

Desktop (please complete the following information):

OS: Arch Linux Browser: Brave Version: 1.57.47

Smartphone (please complete the following information):

sakibian commented 1 year ago

Hey @DenverCoder1 Can I work on this? I don't know If I can solve this, still wanna take this as a challenge.

DenverCoder1 commented 1 year ago

@sakibian Sounds great! Let me know if you have any questions.

It seems like what needs to be done is make sure that first available option (that is not disabled) gets selected in the select element when a property is removed.

sakibian commented 1 year ago

Hey @DenverCoder1 this error is not only happening to the last option field, but it's also happening to other options as well,

screenshotAttached

I think we need to come up with some logic where the disabled option gets hidden when the length is 0 or deleted from the list, showing only those options that are not disabled.

DenverCoder1 commented 1 year ago

I think the problem is that the disabled options are still able to be added when they are still selected.

If we can make it deselect the disabled option after an option is removed, that should fix it.

We have some code in the addProperty function to do this: https://github.com/DenverCoder1/github-readme-streak-stats/blob/1af2a506b6713eac10fd3ffd2dc7c124a398a05b/src/demo/js/script.js#L79-L82

If we get these couple lines to run when removeProperty is called, it might resolve it (we don't need to check firstAvailable exists because the removedProperty will for sure be available)

On second thought, if we already have a reference to the option that was just removed and un-disabled, we can just make that option the selected option because it is available to be added.

Arzzam commented 1 year ago

Is this issue is still exist? Can I take this issue?

sakibian commented 1 year ago

@DenverCoder1 I've got a few issues I need to resolve first, you can unassign me and assign new contributors to this issue, and I'll track your issue list and contribute to other ones.

Arzzam commented 1 year ago

Can I work on this issue? @DenverCoder1

DenverCoder1 commented 1 year ago

@Arzzam Sounds good to me. 👍

This is probably a 1-line fix.

Arzzam commented 1 year ago

Ok thank you