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

Simplify loading-animation icon #407

Closed
jbalsas opened this issue Jan 11, 2018 · 3 comments
Closed

Simplify loading-animation icon #407

jbalsas opened this issue Jan 11, 2018 · 3 comments
Labels

Comments

@jbalsas
Copy link
Contributor

jbalsas commented Jan 11, 2018

Currently, the loading spinner used in Liferay Portal uses the following markup:

<div class="loading-animation"></div>

The current implementation, however, requires the following markup:

<span class="loading-icon loading-icon-dotted">
    <span class="loading-icon-indicator"></span>
</span>

@marcoscv-work did some POC on what it meant updating it in https://github.com/jbalsas/liferay-portal/pull/937/files, and I don't think we can afford that amount of changes.

Is there an option to reduce our current implementation into the simpler markup?

@pat270
Copy link
Member

pat270 commented Jan 11, 2018

@jbalsas I think this means we can't change the markup at all? I took a quick look at @marcoscv's pull and I think that should work if that is the case. I'll let you know.

@pat270 pat270 added the 2.x label Jan 11, 2018
@jbalsas
Copy link
Contributor Author

jbalsas commented Jan 12, 2018

I think, for this particular case it would be ideal if we can keep it the way it is to minify the upgrade efforts.

  • Is there a strong reasoning for it to change? I assume the older format (just a class on any element) is easier to apply to any project than a specific loader markup.
  • Would it make sense to support both markups for the 2.x version?

@pat270
Copy link
Member

pat270 commented Jan 12, 2018

The main reason for the extra markup was to allow the loading icon to size based on font size, but using the after pseudo element solves that. I also thought we should change the component name to describe the animation better, but it's not really that important.

@pat270 pat270 closed this as completed in 2ca3212 Jan 12, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants