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

[Feature] Add fade out alarm option #2813

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

[Feature] Add fade out alarm option #2813

wants to merge 2 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Oct 10, 2017

Very new to electron so please let me know if you would like anything changed.

Cheers :)

P.S. Awesome project! Thanks so much for your time and effort in creating this, has really saved my life at work and home.

@welcome
Copy link

welcome bot commented Oct 10, 2017

💖 Thanks for opening this pull request! 💖
Here is a list of things that will help get it across the finish line: - Run npm run lint locally to catch formatting errors earlier. - Include tests when adding/changing behavior. - Include screenshots and animated GIFs whenever possible. - If you added new translation strings ensure that you have added empty strings for all languages - If you added new functionality or modified existing functionality please ensure it is tested

@ghost
Copy link
Author

ghost commented Oct 11, 2017

Would it be possible to have an option for a sleep timer? Something you can set either by timer or by time of day (i.e. 15 mins or 10:00pm.) It would also be nice if there was an option for a fade out so when the music fades, it's not a hard cut, but instead a nice fade (in which you can set the duration.)

Features still missing from branch:

  • Timer for seconds/minutes/hours
  • Add the fade duration

Will update the branch once I have added/tested the above features.

+ Add duration tests
@ghost
Copy link
Author

ghost commented Oct 25, 2017

image

Added seconds, minutes and hours inputs. Thought adding a fade duration wasn't really necessary so didn't include it in the PR.

Details in regards to the changes. I changed the props in AlarmModal to states as they better handled UI updates such as clearing the time set when a duration value was changed, or, clearing the duration value when a time was set.

I also modified the time and duration settings to be updated on save of the alarm modal. This will prevent the alarm triggering on change of the seconds duration, waiting for the user to press okay to confirm the time allowed. The scenario this would be most handy is if a user set a duration of a second or two. If the settings value was updated on change then the alarm would trigger instantly not allowing the user enough time to click 'ok' to confirm.

@ghost ghost changed the title Add fade out alarm option [Feature] Add fade out alarm option Oct 25, 2017
@MarshallOfSound
Copy link
Owner

Sorry for the delay on this, I'll take a look this week 👍

@Stanzilla
Copy link

@jarradizzle do you still want to do this? looks like a nice feature! :)

@ghost
Copy link
Author

ghost commented Apr 12, 2018

@Stanzilla all the changes are here for this feature? I just don't think it has been reviewed yet. Please let me know if anything is missing.

@tealshift
Copy link

Dag yo, this feature would be awesome! I'm surprised the PR's been sitting here so long.
(Just discovered this project myself. Great stuff guys!)

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