bastislack / highline-freestyle

Webapp for Highline Freestyle
GNU General Public License v3.0
10 stars 9 forks source link

Trick List - Trick Details #279

Closed bastislack closed 4 months ago

bastislack commented 12 months ago

As a Freestyler, I want to see the information about a trick so that I understand how it works and how I can learn it.

Should...

JulianDietzel commented 9 months ago

I'll give this a shot and work on the page.

JulianDietzel commented 8 months ago

All progress on this is pushed onto feat/rewrite-trick-details.

JulianDietzel commented 8 months ago

Since embedding the videos is a bit of a pain and taking a while here is a brief look at the current state:

Feel free to give feedback if you already have any. What still needs to happen is

What can happen is:

The personal / metadata section (stickFrequency, notes, isFavourite) will be added later, on the bottom of the page in issues #289, #293 and #294.

bastislack commented 8 months ago

Nice, I really like the design with the icons for all the titles! Are you struggling with both youtube and ig embeds? I remember that youtube was more or less smooth but ig was a rough process in the first version of the app.

JulianDietzel commented 8 months ago

I'm glad you like the design, thanks :).

Just simply embedding a YouTube video turned out to be quite simple, when using an iframe embed. The issues I've had initially were 1. sizing the video correctly and 2. getting the video to start and end at the correct timestamps if they are provided for a video.

I've figured out how to get the video to be the correct size but getting the video to start, end an loop correctly is still an issue. I've managed to get the video to start at the correct time and stop ant the correct end time. But if the user then presses the replay button of the embed, the video starts from 00:00 instead of the start time. I would also like for the video to loop automatically like it does in the current version but haven't managed to get that to work.

Before the rewrite I / you used a library for embedding YouTube videos that could take care of all these functions. But after trying two different ones for the rewrite (I can't use the old one anymore because of the switch from React to Vue), I have not found one that first of all even works and secondly provides the needed functionality.

I'm dreading implementing the Instagram embed already, as I remember the code you wrote for the old version and have not found a better way to embed Instagram videos yet. But I guess I will find some kind of way, though I fear that it will probably not be perfect.

JulianDietzel commented 7 months ago

I am very close to finishing this feature. The last thing missing is the following requirement: "Allow navigation to trick overview"

Now I could add a "Back"-Button to the top of the page. But on my first try it does not look all to great visually. There is the back button on Android devices which can accomplish the same thing. The issue with this are iOS devices which don't have a back button. But the navbar is visible at all times and allows the user to click on "Tricks" again which brings them back to the overview. The only downside to this is, that it is not the most intuitive way to do this, since the "Trick" tab will already be active.

@bastislack What do you think. Should I experiment with a back button a little more or are we ok with just having the navbar (and the back button of the OS on Android)?

bastislack commented 7 months ago

I really think we should try to use a back button, anything else is really not very user friendly I would say. Can you share a screenshot of how it looked?

JulianDietzel commented 7 months ago

It is definitely more user friendly with the back button. I tried around with it a little more and ended up with the following header:

Bildschirmfoto am 2024-01-31 um 22 45 26

What do you think of this? If you are happy with it I will open a PR with all my changes.

The header is not sticky yet but since it is not critical for it to be sticky and it is a bit of work to make it actually work in a pleasant way (hide when scrolling down and show again when scrolling up) I'd suggest not implementing this in this issue / PR.

bastislack commented 7 months ago

Nice I think it looks good! Only I think we don't need the Trick Name in the header yet since it's not a sticky header. Because now it looks weird having the trick name show twice very close together.

JulianDietzel commented 7 months ago
Thanks :). Regarding the name in the header. I had the same idea originally but felt that it looked a bit empty without the name there. I came out slightly preferring the version with the name over the one without it. But I am happy to drop the name if you prefer the version without the name. Here is how it looks with and without the name side by side. Please let me know which one you like better. name no name
JulianDietzel commented 7 months ago

I've merged the PR (#334) for this issue. I'll leave this issue up though until the question about the name / no name in the header is resolved. Depending on what we end up with I'll add the very small change via a new and equally small PR.

(This also means that if you want to see how the page (and the header) looks and feels when actually running it you can now just pull rewrite and open a route like /tricks/official/3)

JulianDietzel commented 4 months ago

Implemented in #334