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

WIP: Add adjustable skip amount #3113

Merged
merged 36 commits into from
Jul 12, 2024

Conversation

glorenzen
Copy link
Contributor

Adding playback settings to allow for adjusting the amount of time that can be skipped forwards and backward, the same as the mobile app.

This is currently a work in progress, and I've only made a few UI changes so far. It's set up similarly to the mobile app, but I'm wondering if it makes sense to use the ToogleBtns component instead of the IconBtn to allow for more options, as is mentioned in #1995 . The only issue with that is that Material Icons only has icons for up to 30s, so I'm not sure how higher than 30s could be displayed in the audio player.

Resolves #1722 , #1995

@advplyr
Copy link
Owner

advplyr commented Jul 3, 2024

The issue with this is the mobile apps are storing the skip amounts on the device whereas this method would be storing the skip amounts on the server.
I think this setting is best left at the client level otherwise users expect that setting to be respected in all clients.

I'm not sure where it would go exactly which was the initial hang up with that feature request. I was going to add a context menu to the audio player to show more options like the new share feature I'm working on. I suppose there could be a "Settings" option in the audio player that brings up a modal for configuring that. I'm not sure what else would go in there.

I'm open to discussing best user experience for this. Let me know what you think

@advplyr
Copy link
Owner

advplyr commented Jul 3, 2024

If we were to go the route of an audio player settings menu then the context menu icon could replace the "Use chapter track" and the "Use chapter track" setting could go inside the audio player settings menu.

image

With a lot of these feature requests the more difficult part is the UI/UX and once that is figured out the code is straightforward.

@glorenzen
Copy link
Contributor Author

Yeah, I think you're right. Adding it to the settings page may not make the most sense. I do like the idea of the popup menu for the player though. Definitely would make more sense there.

I'll brainstorm the context menu idea a bit.

@glorenzen
Copy link
Contributor Author

Here are a couple of quick mockups that I put together for this.

I replaced the chapter track icon with a separate settings icon for the playback settings, which opens up a modal where the skip and chapter track settings are located. I also swapped out the two forward and replay icons to be more general so that it can accommodate more than just the three icons available in Material Icons. That way there are more options for skipping.

Thoughts?

Player Mockup
Modal Mockup

@glorenzen
Copy link
Contributor Author

Another visual change that could be made that is related to this, is center-aligning the play button and the chapter name below it. It's a small visual change but would improve the overall look of the player.

That could be solved if the playback settings icon is moved to the left of the previous chapter button, or if the playback speed button is moved to the player settings modal.

@glorenzen
Copy link
Contributor Author

@advplyr I've added the modal for the player settings and moved the useChapterTrack and time jump settings to it. When you get a chance, let me know what you think.

One thing I've noticed is that some of the Material Icons that I put in my mockups are not available. Are the woff files an older version of Material Icons?

@glorenzen glorenzen mentioned this pull request Jul 10, 2024
@glorenzen glorenzen marked this pull request as ready for review July 10, 2024 23:26
@advplyr
Copy link
Owner

advplyr commented Jul 11, 2024

I'm not sure what happened with the en-us.json file. I don't know why it is saying the entire thing has changed when the only thing I see is the newline at the end of the file was removed.

What code editor are you using? There is vscode settings file and a .editorconfig file that doesn't appear to be getting applied with your editor.

@glorenzen
Copy link
Contributor Author

I'm using VSCode. I had one of the strings in the wrong order, and when I moved it to alphabetize the keys, it showed almost the entire file as changed.

@advplyr
Copy link
Owner

advplyr commented Jul 11, 2024

Can you check that your VSCode is using the workspace settings instead of the user settings?

Some of the settings are using the vetur extension which should pop up as a recommended extension (one not being applied is trailingComma).
The .editorconfig should automatically add a newline at the end of the file on save.

So it seems these workspace settings aren't being applied

@glorenzen
Copy link
Contributor Author

What's the easiest way to confirm that? I checked the settings, and I can see that some workspace settings are overwriting my User settings. I have the default tab size as 4, but the workspace settings are showing as 2, and when I save, Prettier formats files to use 2 spaces.

I also have the Vetur extension installed.

@glorenzen
Copy link
Contributor Author

I'm unsure what the issue was, but I think it's okay now. I reverted the commit and pushed the changes to the en-us.json fil again, and I'm only seeing a couple of changes in the diff now.

@advplyr
Copy link
Owner

advplyr commented Jul 12, 2024

I ended up adding a new UI component for a dropdown that can overflow the modal. Eventually I would like to clean up all the dropdown components to use one.

I changed 1 minute to display as 60 seconds just to avoid having to add a translation for both singular and plural for minutes.

Thanks!

@advplyr advplyr merged commit 43b7ccd into advplyr:master Jul 12, 2024
5 checks passed
@glorenzen glorenzen deleted the adjustable-skip-amount branch July 15, 2024 21:42
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.

[Enhancement]: Adjustable skip amount
2 participants