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 a low feedback mode #967

Merged
merged 16 commits into from Dec 9, 2023
Merged

Conversation

tonyedwardspz
Copy link
Contributor

@tonyedwardspz tonyedwardspz commented Nov 30, 2023

This pull requests implements the functionality mentioned in #966

After toggling the mode on through the "user interface settings" section of the settings menu, the following has been changed:

  • Download confirmation dialog for podcasts are skipped
  • Download success toasts for podcast are disabled
  • Ditto both the above for audiobooks
  • Downloaded media link status on podcast pages are hidden

I'm not set on any of this and expect it to change. The word choices can probably be vastly improved.

Happy to take feedback to shape this feature. If it expands much beyond a few simple prompts and dialogs, it might be wise to group things under a couple of settings toggles.

Low feedback setting tiggle

@Grog02
Copy link

Grog02 commented Nov 30, 2023

Brilliant idea! I would consider using a word other than feedback for the low feedback mode as it can be confusing having the same word twice but having it mean separate things for some users. Other than that I love the idea of being able to opt out of being shown details on absolutely everything that is going on!

@nichwall
Copy link
Contributor

What if instead of adding checks for every toast, a wrapper function is written for the toast success function? This wrapper would accept a second argument for isLowFeedback and handle that if statement.

That would reduce the number of checks and just be a global option for success toasts. I think the error toasts should not be disabled in Low Feedback Mode yet, but could be in the future.

This could also be extended to groups, since the wrapper toast function would just accept the relevant setting (for the toast) instead of a global Low Feedback Mode.

@nichwall
Copy link
Contributor

nichwall commented Nov 30, 2023

On that note, maybe the "Low Feedback Mode" should just be "Enable Success Messages", and defaults to Enabled.

It should not be called "Disable [item]" because naming settings Disable caused a lot of confusion with other settings in the past (since you ENable to DISable something)

@tonyedwardspz
Copy link
Contributor Author

tonyedwardspz commented Nov 30, 2023

Nice spot @Grog02

Good idea on the wrapper @nichwall. I'll look into that.

Agree with keeping error messages generally speaking, but reserve the right to sneak a select few through :). I'd wanna assume things go right, but know when they go horribly wrong.

I wouldn't be opposed to avoiding disable wording. "Enable Success Messages" makes sense if we sperate that functionallity out as its own setting. "Enable Reduced Feedback" probably better describes the setting for the current implementation in that context.

@tonyedwardspz
Copy link
Contributor Author

I've pondered the wrapper function.

Can someome point me to an appropriate central place in the code base to put the function please?

I'll be able to work backwards from there, just don't have my head fully around the structure quite yet.

@nichwall
Copy link
Contributor

nichwall commented Dec 1, 2023

My guess would be plugins

@advplyr
Copy link
Owner

advplyr commented Dec 3, 2023

I actually think it's fine if we remove the download success toasts. The pattern so far has been I overuse the toasts in the beginning and they slowly get trimmed back.
Are there other toast messages you would like to see hidden? Maybe those are fine to also remove

I'm not sure about hiding the alert messages about local media not being syncable to the server. I can foresee debugging with someone in the future about a sync issue come to find out they had that hidden. Maybe we don't show the successfully linked alert box anymore and when not connected to a server we also don't show anything.

@nichwall
Copy link
Contributor

nichwall commented Dec 4, 2023

I'm not sure about hiding the alert messages about local media not being syncable to the server. I can foresee debugging with someone in the future about a sync issue come to find out they had that hidden. Maybe we don't show the successfully linked alert box anymore and when not connected to a server we also don't show anything.

I agree with never hiding the connected to a different server/user message since people are still confused by how multiple servers work and the website guides are not translated at all (the apps supporting localization will help with this confusion). It also provides an easy way to remember which of the several servers/users you used to download media if that's applicable, and if someone is only using one server/user then they'll just never see it anyway.

I think keeping the "Linked to server [server]" should remain in offline mode but could be hidden by the setting. Or, maybe only show this message if there's more than one server connection?

I think the "connected to the correct server" could be removed entirely, though. It's nice to have but I agree it's not necessary.

@tonyedwardspz
Copy link
Contributor Author

tonyedwardspz commented Dec 5, 2023

Are there other toast messages you would like to see hidden? Maybe those are fine to also remove

The others were linked to server sync status, but I can't think when they pop up cause I've not seen them for a few days.

After thinking, I was leaning towards two toggles:

  • Hide success toasts
  • Hide media link status

The latter would hide the main server link message on the podcast page, along with related toasts.

My issue with the main media link message connects back to its positioning and coloring. The message is currently presented as the most important piece of info on the page, but it's not for many users (single server / not a stats tracker).

I do like the suggestions tho. Removing it when possible would help, and hiding it under certain conditions (e.g offline) would be good. If we're going to not allow hiding it totally, can the status be better blended into the UI? What about condensing the message into another line under Author / Genre? The info is still present, but visually deprioritised.

Screenshot 2023-12-05 at 06 19 41

Layer in the same coloring for the lettering currently used for different states, and user attention will be draw to it when needed but allow it better blend in when not. Hiding it dependant on state will cause a less noticable variation in the UI with this layout.

Even with moving the info, I'd still push for it to be present as little as possible, particuarly when everything's ok / app is in disconnected state as you both discuss. It'd still be good to hide it totally tho, and that would be my personal preferance.

@nichwall
Copy link
Contributor

nichwall commented Dec 5, 2023

I agree that presenting the information in a less visible way would be nice, but there was more confusion when this was less prominent. I'm not sure when you joined the project, but this is how it used to look. The new messages were added in 0.9.67. (I covered up my URL, but it was in the same color of white as the other text)

Screenshot_20231023-124302.png

Screenshot_20231023-144429.png

The issue was people kept getting confused and posting in Discord and GH about how sync was broken, it doesn't work, etc, but they were listening without connecting to the server so it couldn't ever sync back. Then, they'd listen on another device, and then their progress would get messed up. These new messages were an attempt to make it more visible that they were not connected, but also to provide information to users who don't understand the functionality.

I think it still needs to be more prominent. Maybe the setting could toggle between the big message with the warning and the little message without a warning, and have it on the big message by default? I don't want people just turning it to low priority things and then getting confused why things don't work. Answering those questions all the time because the information wasn't readily available in the app and people wouldn't search GH or Discord before asking took a lot of time.

@advplyr
Copy link
Owner

advplyr commented Dec 5, 2023

I think the media link alerts can be addressed without requiring a toggle setting. The download success toasts can be removed also. Any other unnecessary toasts could be removed if they are found.

The only thing a setting may be useful for is the confirmation that comes up when clicking download. For podcast episodes we may not need the confirmation since the files are smaller. Audiobooks should keep the confirmation on download.

Since your initial request is related to podcast episode downloads I wonder if a setting is necessary at all if the above is updated.

@tonyedwardspz
Copy link
Contributor Author

tonyedwardspz commented Dec 8, 2023

This sounds like a good evolution of the UI taking in the discussion 👍. Happy to move my branch towards this and give it a go.

@nichwall I've been around for a few versions of the Android app. I remember the previous version of the media link notification, but never totally understood what it was tbh. As I'm not fussed about stats/syncing, there was never a problem regardless of what it said, but knew it was somehow connected syncing. It was something I could safely ignore rather than have to think about every time I open a podcast.

I totally get the frustration of repeated questions in discord. I've lived that admin life. I found it was one of the trade-offs of using a chat platfrom as a knowledge repository mixed with a community hub. It biases towards chat rather than retreival to solve problems. There is a fabulous community hanging out there tho 💪

Through this discussion, I've realised that my OP could have been more focused around stats/syncing. A toggle to say stats don't matter, I don't want to know when sycncing goes wrong. Framing a toggle around that would likely avoid confusion about what the toggle does. That would be sperate to this tho.

@tonyedwardspz
Copy link
Contributor Author

tonyedwardspz commented Dec 8, 2023

Based on our conversation (thanks for taking the time to have it btw), my branch currently:

Toasts / Prompts

  • The download success toasts have been removed
  • Podcast downloads don't create a confirmation dialog
  • Audiobook download confirmation dialog remains

Media link status

  • Removed the successfully connected media link messages
  • Hides the media link status if not connected to the server

This setup seems to work for me with the limited use so far.

I've also commented out a few toasts related to succesfully deleting things. These would be my suggestions for the moment.

Others are tightly coupled to media sync status and are warning/error messsages. It only makes sense to remove these for users who explicilty choose to ignore the stats part of the app ( and therefore won't end up is discord asking for help with syncing ). That would suggest a toggle is needed, but is outside of the scope of this pull imo.

@nichwall
Copy link
Contributor

nichwall commented Dec 8, 2023

Based on a comment from Sapd in Discord, what if instead the media link info box changes to a popup and was formatted like before?

Instead of showing a green "connected" or the white IP, instead always show the IP/server connection, with a green "Connected" or red "Not Connected" directly below it. There would then be an info bubble next to the connection state that users can tap on to get the messages currently displayed as a popup. Those could be color coded if desired, but I don't think the color coding would be necessary.

That would still provide information and make it less intrusive. The info bubble would also make it more clear that this is useful sync information versus a string without a label.

@tonyedwardspz
Copy link
Contributor Author

tonyedwardspz commented Dec 8, 2023

For the purposes of my request, my commits do the job and think it's a happy medium for now after a few hours use. It removes feedback from the happy path, leaving it available for anyone not on it. . . including me :)


But if we had to. . . how about condensing that suggestion onto one line? The flow would be:

(i) Server IP - Sync Status

Clicking any of it brings up the dialog/bubble, with the same info i (???) that's used in the settings area to indicate that there is more info to be discovered (I think this is exactly what you mean).

Keeping it on one line better connects the IP to the connection status.

We'd be trusting users to click on it, and know they have to click on it, when they have problems they possibly don't fully understand and that don't obviously link to this feedback on the face of it (I've always assumed a good chunk of people who are setting ABS up are fairly new to the world of ip's / servers / linux etc). The current release is much clearer as to what the message means than the last one, and I think this implementation would hide that useful info for users who are stuck. It's the unmissabe info in the current version that is likely cutting chat down.

The same would apply to putting the whole message behind an icon next to the title. Whist personally preferable, this could hide the info even more. Agree that warnings shouldn't be hidden. . . unless the user wants to hide them and they're non-critical. This was the thinking behing the toggle.

If we did one of these ways, I'd be interested to know if there is an uptick of support requests after release. I'm not sure if we should tho.

There's an elegant solution in here somewhere :) I'm happy with my rejig for now tho.

@advplyr
Copy link
Owner

advplyr commented Dec 8, 2023

I realized while testing this that the latest page was never fully setup with local downloads. I'll have to fix this before merging because without the toast there is no indication that the episode was downloaded.

@advplyr
Copy link
Owner

advplyr commented Dec 9, 2023

This is good to go now. We will only show an alert now if the user is connected to a server and it is not the same server as the downloaded book they are viewing. We can make further adjustments if we need to with more feedback.

If we can continue to remove unnecessary toasts and alerts we definitely should, so hopefully we don't need a setting for this. The less settings and noise the more intuitive the app should be hopefully.

Thanks!

@advplyr advplyr merged commit 5270104 into advplyr:master Dec 9, 2023
1 check passed
@tonyedwardspz
Copy link
Contributor Author

Brilliant. Thank you for accepting this. I'm really pleased that my first ever open source pull is for ABS 🙌

I really appreciate everyone's time in helping me put this together.

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.

None yet

4 participants