Skip to content

Fix the inifite carousel text.#8

Open
LeonardRuhmann wants to merge 7 commits into
Lucas-BRT:developmentfrom
LeonardRuhmann:development
Open

Fix the inifite carousel text.#8
LeonardRuhmann wants to merge 7 commits into
Lucas-BRT:developmentfrom
LeonardRuhmann:development

Conversation

@LeonardRuhmann

Copy link
Copy Markdown
Contributor

There is an issue with the text size. When the text size is too large, the carousel breaks before the text is fully scrolled. To address this, each text now has its own carousel. Additionally, different carousel types have been added: small for small text, medium for medium text, and large for large text.

Before:
0528

Now:
now

@Lucas-BRT

Copy link
Copy Markdown
Owner

some audio tracks are simply disappearing in the new version, check what is happening.

@LeonardRuhmann

Copy link
Copy Markdown
Contributor Author

It's because of the GIF. Check out this new GIF below.

0529.mp4

@Lucas-BRT

Copy link
Copy Markdown
Owner

it's missing the fade-in on entry and exit of the carousel

@LeonardRuhmann

Copy link
Copy Markdown
Contributor Author

Fade-In and Fade-Out is working.

before:

before

after:

after

@Lucas-BRT Lucas-BRT left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

the code have a significant amount of bugs and unexpected behaviour, please fix it for progress in the PR

const textElement = playbackbar.textContent;

switch (true) {
case textElement.length > 21 && textElement.length < 40:

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

this function not cover if the length is equal 40

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed whitin the next commit

Comment thread src-mobile/index.html
<div class="navegation-background">
<div class="navegation-button actived">
<h1>Tracks</h1>
<head>

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

it's missing the icon declaration

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed whitin the next commit.

@@ -0,0 +1,29 @@
import { putFade } from "./putFade.js";

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

this function is redundant, the checkPlaybackbarTextAndAddCarrousel have the same purpose

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed whitin the next commit

// Function to update the carousel based on current time and animation duration
const updateCarousel = () => {
const currentTime = Date.now();
const elapsedTime = (currentTime - animationStartTime) % animationDuration;

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

why are you getting the module of this division?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

the updateCarousel is calculating the elapsed time since the start of the animation. We achieve this by subtracting animationStartTime (the time when the animation began) from currentTime (the current time). The module operator is used to obtain the remainder of the division by animationDuration, ensuring that elapsedTime always falls within the range of 0 to animationDuration.

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