Skip to content

Added initial implementation of loader - spinner#1631

Open
nitinja wants to merge 4 commits into
developfrom
component/vf-indicator-loader
Open

Added initial implementation of loader - spinner#1631
nitinja wants to merge 4 commits into
developfrom
component/vf-indicator-loader

Conversation

@nitinja

@nitinja nitinja commented Jul 22, 2021

Copy link
Copy Markdown

No description provided.

@nitinja nitinja requested a review from khawkins98 July 22, 2021 10:57
@nitinja

nitinja commented Jul 22, 2021

Copy link
Copy Markdown
Author

I am working on eleventy page demo & react demo for this component

@netlify

netlify Bot commented Jul 22, 2021

Copy link
Copy Markdown

✔️ Deploy Preview for ecstatic-noether-150ab4 ready!

🔨 Explore the source changes: 540597e

🔍 Inspect the deploy log: https://app.netlify.com/sites/ecstatic-noether-150ab4/deploys/612ce107e240fd0007e2e9c2

😎 Browse the preview: https://deploy-preview-1631--ecstatic-noether-150ab4.netlify.app

@nitinja

nitinja commented Jul 22, 2021

Copy link
Copy Markdown
Author

Eleventy page showing preview correctly in dev mode, but not on deployed preview
image

@sandykadam

sandykadam commented Jul 22, 2021

Copy link
Copy Markdown
Collaborator

@khawkins98 @nitinja just curious to know if we can build our own EMBL style loader/spinner instead of standard one.

Like we can use the hexagon icon of logo and circle around it animated effect.

https://monophy.com/gifs/embl-embllogo-embllogomonochrome-SU8RkgEO4bcb9bL1U9

https://www.embl.org/news/wp-content/uploads/2014/09/embl_virus_logo_animation_5sec.gif

@nitinja

nitinja commented Jul 22, 2021

Copy link
Copy Markdown
Author

Animation

@sandykadam

Copy link
Copy Markdown
Collaborator

Ah looks nice 👍

Comment thread components/vf-indicator-loader/README.md Outdated
Lerna will bump this to alpha.1 on release

<div class="vf-indicator-loader--spinner__circle"></div>

{% if example == true %}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

further above you used {% if container == true %}... did you mean to container here as well?

.vf-indicator-loader--spinner__circle {
width: 32px;
height: 32px;
color: $vf-indicator-loader--border-color;

@khawkins98 khawkins98 Jul 27, 2021

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I have some suggestions on how to use the design tokens here. Might be clearest/best if I just do that and then we can look at the diff.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@nitinja I still need to update this Sass, but if you could please look at the other PR comment.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I've addressed this with commit 0c370a6

@khawkins98

Copy link
Copy Markdown
Contributor

Looking good @nitinja — I did have a few minor things that I've noted.

@sandykadam

Copy link
Copy Markdown
Collaborator

Looking good @nitinja — I did have a few minor things that I've noted.

Are we good to use hexagon style or grey circle loader

@nitinja

nitinja commented Jul 28, 2021

Copy link
Copy Markdown
Author

@sandykadam I have committed just the circle spinner as of now, I think we can't commit any design that's not approved by UX people.

This does a couple of things:

1. Sass linting to alphabitise ordering
2. use CSS vars (custom props) — we tend to use this over the Sass vars as this allows more possibility to cascade in new values for styling
@khawkins98

Copy link
Copy Markdown
Contributor

Adding a note here that based on our recent conversation with @cindyebi and @nitinja this will be part of vf-progress-indicator, and specifically a vf-progress-indicator--spinner variant.

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.

3 participants