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

feat: Add loader #1180

Merged
merged 9 commits into from
Dec 21, 2024
Merged

feat: Add loader #1180

merged 9 commits into from
Dec 21, 2024

Conversation

JulienDeveaux
Copy link

@JulienDeveaux JulienDeveaux commented Dec 12, 2024

src/main.js Outdated Show resolved Hide resolved
@ildar170975
Copy link
Collaborator

@akloeckner
Can you have a look now?
We added a small fix related to css. In other things I am not 100% confident, we need to test )))

If OK - then please as usual:

  • generate js
  • we will test it together
  • create Release if OK

@JulienDeveaux
Copy link
Author

I've removed an extra check and the feeling of the loader is better and more consistent

@JulienDeveaux
Copy link
Author

I'm changed the loader from the pulsating circle to this pulsating graph :
image

I think the circle wasn't good enough as it wasn't showing a graph
Tell me what you think, I'll roll back it if necessary

src/main.js Outdated Show resolved Hide resolved
Copy link
Collaborator

@akloeckner akloeckner left a comment

Choose a reason for hiding this comment

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

I took the liberty to restart the minor cleanups in separate comments. I felt, it became a bit mixed up in the one long conversation we're having.

src/main.js Outdated Show resolved Hide resolved
src/main.js Outdated Show resolved Hide resolved
src/main.js Outdated Show resolved Hide resolved
src/main.js Outdated Show resolved Hide resolved
@akloeckner
Copy link
Collaborator

😆 Nice commit title...

From the theoretical standpoint, this PR looks really great to me now. I'll download and test it again in the coming days.

@ildar170975
Copy link
Collaborator

Seems great.
I would even suggest to merge it)))

@ildar170975
Copy link
Collaborator

@JulienDeveaux
@akloeckner
Really enjoyed working together.

@akloeckner
Copy link
Collaborator

I would even suggest to merge it)))

Did you test it? I would then trust you and merge it.

@ildar170975
Copy link
Collaborator

ildar170975 commented Dec 20, 2024

Did you test it?

Yeah, a little))))) Will not cause a nuclear war I hope.
Progress indicator is displayed fine.

@akloeckner
Copy link
Collaborator

Thanks, guys. This was really a productive night!

@akloeckner akloeckner merged commit 30c5263 into kalkih:dev Dec 21, 2024
4 checks passed
github-actions bot pushed a commit that referenced this pull request Dec 21, 2024
# [0.13.0-dev.1](v0.12.2-dev.2...v0.13.0-dev.1) (2024-12-21)

### Bug Fixes

* name and icon color respect attribute choice ([#1131](#1131)) ([cbfba7a](cbfba7a))

### Features

* Add loader ([#1180](#1180)) ([30c5263](30c5263))
* add show_legend_state ([#1173](#1173)) ([3b1c827](3b1c827))
Copy link

🎉 This PR is included in version 0.13.0-dev.1 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants