fix(app): Move random mode to navigation menu. #559

robercoding · 2021-03-13T09:27:08Z

Changes

Moved random mode to navigation menu!

Others

Resolves make random mode more prominent #537

robercoding · 2021-03-13T09:27:51Z

I should've added a UI Test. I'll do it in a moment.

codecov · 2021-03-13T09:35:51Z

Codecov Report

Merging #559 (b754101) into master (077eca3) will increase coverage by 0.12%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           master     #559      +/-   ##
==========================================
+ Coverage   47.78%   47.90%   +0.12%     
==========================================
  Files          33       34       +1     
  Lines        1871     1866       -5     
  Branches      180      179       -1     
==========================================
  Hits          894      894              
+ Misses        930      925       -5     
  Partials       47       47

Flag	Coverage Δ
android	`44.45% <0.00%> (+0.12%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...ava/com/github/ashutoshgngwr/noice/MainActivity.kt	`0.00% <0.00%> (ø)`
...hutoshgngwr/noice/fragment/RandomPresetFragment.kt	`0.00% <0.00%> (ø)`
...hutoshgngwr/noice/fragment/SoundLibraryFragment.kt	`0.00% <ø> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 077eca3...b754101. Read the comment docs.

ashutoshgngwr

Sorry if I wasn't clear earlier, but the intention was to remove the dialog fragment as well and make the random preset fragment like the rest of the screens, e.g. sleep timer.

Wouldn't require changing layouts. Would require to add a new fragment and move existing tests from SoundLibraryFragment.

robercoding · 2021-03-13T10:31:06Z

Oh that makes more sense, I'll change and adapt to that solution which I think it's much better.

robercoding · 2021-03-13T18:03:00Z

Hey @ashutoshgngwr I started doing it now again, and I have some doubts as it seems pretty easy it's also a bit confusing.

I understand that you want a fragment for random presets that is accesed by navigation menu. Once user click there, what is he expected to see on that fragment screen?
e.g A blank screen(yet) with a fab button, then click there and a dialog fragment shows up? Like the same random functionality we had on the previous screen but on a new fragment?

PS: I already created a Fragment and user can acceess there but moving test from SoundLibraryFragment to the new RandomPresetFragment test requires more changes to make them work.

Thank you!

ashutoshgngwr · 2021-03-13T18:13:14Z

I understand that you want a fragment for random presets that is accesed by navigation menu. Once user click there, what is he expected to see on that fragment screen?

Essentially the same layout that was shown in the random preset dialog. When a user clicks the random preset menu item, they are taken to a fragment with the following form.

PS, We can make the play button a floating action button. We can also move and expand the highlighted text to the top like other screens, e.g. Generated random presets will contain sounds corresponding to the chosen mood. Moreover, dense presets will attempt to play more sounds simultaneously.

PS: I already created a Fragment and user can acceess there but moving test from SoundLibraryFragment to the new RandomPresetFragment test requires more changes to make them work.

Okay, first let's get everything else right. We will worry about the tests later.

Let me know if I can help you with anything else.

….kt.

robercoding · 2021-03-13T18:57:52Z

Tell me if you like the changes and what do you want we continue working on!
I just added the basics to make sure I'm doing exactly we're looking for.

Let me know how you're looking at them!

ashutoshgngwr

I also didn't see any deletions in SoundLibraryFragment. I am guessing you left it out for last.

ashutoshgngwr · 2021-03-14T04:37:03Z

app/src/main/res/layout/random_preset_fragment.xml

+      <Button
+        android:id="@+id/cancel_preset_button"
+        style="?attr/buttonStyle"
+        android:layout_width="wrap_content"
+        android:layout_height="wrap_content"
+        android:layout_marginTop="16dp"
+        android:visibility="visible"
+        android:text="@string/cancel"
+        app:layout_constraintEnd_toStartOf="@id/play_preset_button"
+        app:layout_constraintTop_toTopOf="parent"
+        app:layout_constraintBottom_toBottomOf="parent" />


This isn't really needed...? There's nothing to cancel.

You're right! I just followed the exact same picture layout, but it was still kinda weird for me looking at the buttons there, it looked pretty ugly, and as you said play button could be a fab! that's why I decided to push these changes before continue doing more work.

ashutoshgngwr · 2021-03-14T04:37:18Z

app/src/main/res/layout/random_preset_fragment.xml

+      <Button
+        android:id="@+id/play_preset_button"
+        style="?attr/buttonStyle"
+        android:layout_width="wrap_content"
+        android:layout_height="wrap_content"
+        android:layout_marginTop="16dp"
+        android:visibility="visible"
+        android:text="@string/save"
+        app:layout_constraintEnd_toEndOf="parent"
+        app:layout_constraintTop_toTopOf="parent"
+        app:layout_constraintBottom_toBottomOf="parent"/>


It can be a floating action button.

app/src/main/res/layout/random_preset_fragment.xml

ashutoshgngwr · 2021-03-14T04:39:12Z

app/src/main/java/com/github/ashutoshgngwr/noice/fragment/RandomPresetFragment.kt

+    }
+
+    binding.cancelPresetButton.setOnClickListener {
+      //What do we do cancel? Should we reset preferences button?


It's not required in this case.

ashutoshgngwr · 2021-03-14T04:40:42Z

app/src/main/java/com/github/ashutoshgngwr/noice/fragment/RandomPresetFragment.kt

+      }
+      MediaPlayerService.playRandomPreset(requireContext(), tag, intensity)
+      //We'll add later a if player is playing then show else there was an error
+      Toast.makeText(requireContext(), "Playing Random!", Toast.LENGTH_SHORT).show()


I think clicking the play button should just play a random preset regardless if the player was already playing. The logic remains the same. MediaPlayerService internally stops the existing playback before starting a random (or any) preset. There's no error scenario here per-say.

ashutoshgngwr · 2021-03-14T04:42:19Z

app/src/main/java/com/github/ashutoshgngwr/noice/fragment/RandomPresetFragment.kt

+
+      // Your choice to show up in-app review on this fragment!
+      // maybe show in-app review dialog to the user
+      //InAppReviewFlowManager.maybeAskForReview(requireActivity())


It doesn't always result in the dialog being displayed. Sometimes it will be displayed or if the user has already completed the flow once, it won't be shown so it's a safe call.

ashutoshgngwr · 2021-03-14T04:44:38Z

app/src/main/res/layout/random_preset_fragment.xml

+    <TextView
+      android:layout_width="match_parent"
+      android:layout_height="wrap_content"
+      android:layout_marginVertical="7dp"
+      android:text="@string/intensity_description" />


Move this to the top of the layout and change the message. The message could be: Generated random presets will contain sounds corresponding to the chosen mood. Moreover, dense presets will attempt to play more sounds simultaneously.

ashutoshgngwr · 2021-03-14T04:45:11Z

app/src/main/res/layout/random_preset_fragment.xml

+      android:layout_height="wrap_content"
+      android:layout_marginBottom="7dp"
+      android:text="@string/mood"
+      android:textAppearance="?attr/textAppearanceHeadline5"


Opinion: This looks ugly in its own fragment. Maybe try headline 6 for both headings.

robercoding · 2021-03-14T08:05:35Z

Hey @ashutoshgngwr, do you like the actual layout? Is the same as before but with implemented changes that you gave me.
Top text Generated random presets... is centered.
Once I finish this I'll continue with sound library fragment deleting its stuff. And moving tests to RandomPresetFragmentTest!

Let me know if there's anything I could improve on!

ashutoshgngwr · 2021-03-14T09:05:11Z

@robercoding Looks good. Can be improved I guess. Other fragments have a padding of 20dp on their root layouts (inside scroll view). I think that would look good.

The FAB doesn't look properly positioned on large screens. I am guessing you didn't wrap everything in Relative or Constraint layout, e.g. Relative/Constraint Layout -> FAB + (ScrollView -> Actual Fragment). The bottom-end corner is the conventional position for FABs.

PS, another thing we could try is to orient the radio buttons horizontally. It is easily achievable using FlexboxLayout and we already have it as a dependency in the timer picker view. The final layout structure would look like RadioGroup -> FlexboxLayout -> RadioButton. Not sure if the results would be good, but definitely worth a shot.

robercoding · 2021-03-14T09:37:20Z

Hey @ashutoshgngwr, I've done these new changes and looks much better now!!

Using Flexbox scenario with that structure layout (RadioGroup -> Flexbox -> RadioButtons) is impossible because RadioButtons are not controlled by RadioGroup and user can select all 3 options at the same time. But there's an easy way to achieve horizontal orientation by just setting radiogroup orientation as horizontal! I second that idea! ✌️

I'll push today as soon as I finish other parts!

ashutoshgngwr · 2021-03-14T09:43:40Z

Using Flexbox scenario with that structure layout (RadioGroup -> Flexbox -> RadioButtons) is impossible because RadioButtons are not controlled by RadioGroup and user can select all 3 options at the same time. But there's an easy way to achieve horizontal orientation by just setting radio group orientation as horizontal!

Well, using horizontal orientation means that any overflow will be clipped off-screen, so that doesn't work either. 😓

On a side note, increase padding between description text and mood heading. It looks a little cluttered right now.

robercoding · 2021-03-14T10:30:57Z

What do you think about using this library for horizontal view? It's built on top of FlexboyLayout!
Library FlexTools

It doesn't overflow if you set more RadioButtons in the future! I couldn't find a way to center its radiobuttons (At least the options I've tried) but setting a minWidth and wrap content in width looks good on both.

Here's a little proof:

It's up to you to use this external library! Let me know if you like it.

ashutoshgngwr · 2021-03-14T10:39:46Z

@robercoding It doesn't look mature, but if the library works, I guess it's fine. If something breaks in the future, we will figure something out. 😅

Everything looks great now! 👍

…et_fragment.xml. Deleted SoundLibraryFragment.kt random preset button and dialog_fragment__random_preset.xml. Changed intensity_description in strings.xml (Only english).

…last commit). Set manually radioButtons since is not working in XML. Remove 2 tests from SoundLibraryFragmentTest.kt and add 1 test to RandomPresetFragmentTest.kt

robercoding · 2021-03-14T16:27:42Z

I've spent time wondering how it works to make play sounds and how it arrives to the UI and to the back until I just remembered you provided us a diagram on CONTRIBUTING.md. My bad there lol.
I did spent time just to know if I could add some more UI Tests, but I don't seem to find anything more to add (maybe I'm wrong), let me know if I'm missing something or is that ok for you!

ashutoshgngwr

Needs a few styling changes, looks great otherwise. Thanks! 👍

ashutoshgngwr reviewed Mar 13, 2021

View reviewed changes

robercoding added 2 commits March 13, 2021 19:20

fix(app): Added RandomPresetFragment.kt

830d5d0

feat(app): Added option to play random mode in a RandomPresetFragment…

87cbeb9

….kt.

robercoding force-pushed the fix-issue-537 branch from 9c81312 to 87cbeb9 Compare March 13, 2021 18:55

ashutoshgngwr requested changes Mar 14, 2021

View reviewed changes

robercoding added 2 commits March 14, 2021 11:51

feat(app): Add FlexRadioGroup Library and improved layout random_pres…

d23c27d

…et_fragment.xml. Deleted SoundLibraryFragment.kt random preset button and dialog_fragment__random_preset.xml. Changed intensity_description in strings.xml (Only english).

feat(app): Add FlexRadioGroup Library (forgot to add build.gradle in …

b73944c

…last commit). Set manually radioButtons since is not working in XML. Remove 2 tests from SoundLibraryFragmentTest.kt and add 1 test to RandomPresetFragmentTest.kt

chore(app): fix formatting, cleanup obsolete string resources

b754101

ashutoshgngwr approved these changes Mar 15, 2021

View reviewed changes

ashutoshgngwr marked this pull request as ready for review March 15, 2021 02:42

ashutoshgngwr merged commit feb0e7e into trynoice:master Mar 15, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(app): Move random mode to navigation menu. #559

fix(app): Move random mode to navigation menu. #559

robercoding commented Mar 13, 2021

robercoding commented Mar 13, 2021 •

edited

Loading

codecov bot commented Mar 13, 2021 •

edited

Loading

ashutoshgngwr left a comment

robercoding commented Mar 13, 2021

robercoding commented Mar 13, 2021

ashutoshgngwr commented Mar 13, 2021

robercoding commented Mar 13, 2021 •

edited

Loading

ashutoshgngwr left a comment

ashutoshgngwr Mar 14, 2021

robercoding Mar 14, 2021

ashutoshgngwr Mar 14, 2021

ashutoshgngwr Mar 14, 2021

ashutoshgngwr Mar 14, 2021

ashutoshgngwr Mar 14, 2021

ashutoshgngwr Mar 14, 2021 •

edited

Loading

ashutoshgngwr Mar 14, 2021

robercoding commented Mar 14, 2021 •

edited

Loading

ashutoshgngwr commented Mar 14, 2021

robercoding commented Mar 14, 2021

ashutoshgngwr commented Mar 14, 2021

robercoding commented Mar 14, 2021 •

edited

Loading

ashutoshgngwr commented Mar 14, 2021

robercoding commented Mar 14, 2021 •

edited

Loading

ashutoshgngwr left a comment

fix(app): Move random mode to navigation menu. #559

fix(app): Move random mode to navigation menu. #559

Conversation

robercoding commented Mar 13, 2021

Changes

Others

robercoding commented Mar 13, 2021 • edited Loading

codecov bot commented Mar 13, 2021 • edited Loading

Codecov Report

ashutoshgngwr left a comment

Choose a reason for hiding this comment

robercoding commented Mar 13, 2021

robercoding commented Mar 13, 2021

ashutoshgngwr commented Mar 13, 2021

robercoding commented Mar 13, 2021 • edited Loading

ashutoshgngwr left a comment

Choose a reason for hiding this comment

ashutoshgngwr Mar 14, 2021

Choose a reason for hiding this comment

robercoding Mar 14, 2021

Choose a reason for hiding this comment

ashutoshgngwr Mar 14, 2021

Choose a reason for hiding this comment

ashutoshgngwr Mar 14, 2021

Choose a reason for hiding this comment

ashutoshgngwr Mar 14, 2021

Choose a reason for hiding this comment

ashutoshgngwr Mar 14, 2021

Choose a reason for hiding this comment

ashutoshgngwr Mar 14, 2021 • edited Loading

Choose a reason for hiding this comment

ashutoshgngwr Mar 14, 2021

Choose a reason for hiding this comment

robercoding commented Mar 14, 2021 • edited Loading

ashutoshgngwr commented Mar 14, 2021

robercoding commented Mar 14, 2021

ashutoshgngwr commented Mar 14, 2021

robercoding commented Mar 14, 2021 • edited Loading

ashutoshgngwr commented Mar 14, 2021

robercoding commented Mar 14, 2021 • edited Loading

ashutoshgngwr left a comment

Choose a reason for hiding this comment

robercoding commented Mar 13, 2021 •

edited

Loading

codecov bot commented Mar 13, 2021 •

edited

Loading

robercoding commented Mar 13, 2021 •

edited

Loading

ashutoshgngwr Mar 14, 2021 •

edited

Loading

robercoding commented Mar 14, 2021 •

edited

Loading

robercoding commented Mar 14, 2021 •

edited

Loading

robercoding commented Mar 14, 2021 •

edited

Loading