Skip to content

Temporary workaround for app crashing#5

Merged
certified84 merged 5 commits intocertified84:v0.1.4from
gdonisi:v0.1.4
Feb 25, 2022
Merged

Temporary workaround for app crashing#5
certified84 merged 5 commits intocertified84:v0.1.4from
gdonisi:v0.1.4

Conversation

@gdonisi
Copy link
Copy Markdown
Contributor

@gdonisi gdonisi commented Feb 22, 2022

The app is basically useless if it crashes before saving a note. This should fix #2 and gives you time to find a better solution if needed, because probably this isn't a perfect code style.
Tested with these languages as system default: English, Italian, French, German.

@gdonisi
Copy link
Copy Markdown
Contributor Author

gdonisi commented Feb 23, 2022

I've added a new commit. This fixes the app crash when a user clicks the Save button when he don't record the note. Also fixes a blank view if a user records a 0 seconds audio note:
0 second note

@gdonisi
Copy link
Copy Markdown
Contributor Author

gdonisi commented Feb 23, 2022

Third commit, I think using a timestamp instead of the note title as filename is better, I have two main reasons:

  • Title can be added later: if someone's in a hurry and just wants to record a quick note, he just starts recording and he can add the title before saving. Now the user can also modify the note title without consequences.
  • Saving two or more notes with the same title would overwrite the previous note. Having a timestamp prevents accidental loss.

Copy link
Copy Markdown
Owner

@certified84 certified84 left a comment

Choose a reason for hiding this comment

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

Hi there @gdonisi,

Well done so far. Saving a recording with the current time instead of the note title was always something I considered working on and I'm happy you already did. I'm also happy you found a better way of handling the roundOffDecimal() function.

Before merging this PR, I'd like to understand why you changed all the requireContext() to context.

@gdonisi
Copy link
Copy Markdown
Contributor Author

gdonisi commented Feb 25, 2022

Sure, it's just my style of coding: when I make a lot of calls to the same function like this, I prefer to have one variable to store the value (the context in this case) and use it instead of calling always the function.
Nothing changes to the behavior, you can use the requireContext again if you wish to 😁

@certified84
Copy link
Copy Markdown
Owner

Its fine since there are no changes to the behavior. Well done mate 🙌🙌
I'll be merging the PR now.

@certified84 certified84 merged commit 5f0e17d into certified84:v0.1.4 Feb 25, 2022
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.

2 participants