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

Progress #78

Closed
1 task done
eddywashere opened this issue Jul 31, 2016 · 3 comments · Fixed by #107
Closed
1 task done

Progress #78

eddywashere opened this issue Jul 31, 2016 · 3 comments · Fixed by #107

Comments

@eddywashere
Copy link
Member

eddywashere commented Jul 31, 2016

http://v4-alpha.getbootstrap.com/components/progress/

  • Progress
    • animated
    • striped
    • color
@eddywashere eddywashere added this to the Stable Release milestone Jul 31, 2016
@TheSharpieOne
Copy link
Member

While this component is relatively simple, the problem lies with HTML5, bootstrap, and their being different way to render what is essentially the same thing.
I am all for using HTML5 components, but for better backwards compatibility at no real cost; I would use the div elements over progress. Even for modern browsers (as in, do not wrap the div within a progress element).
Animation doesn't seem to work on most modern browsers anyways (also, this comment from the linked issue may suggest that bootstrap will itself go back to just the div)

What do you think?

@eddywashere
Copy link
Member Author

Nice find! I tried creating a div version without the progress element wrapping it and it didn't seem to show a progress bar at all. If it does work without progress, that sounds like a better option. Otherwise, I think sticking to the progress element is not so bad since it's broken in bootstrap and can be fixed on their end. If the markup does change the component can be updated without people having to worry too much about what's changed between bootstrap releases.

@TheSharpieOne
Copy link
Member

TheSharpieOne commented Aug 18, 2016

I forgot I had add some custom styles to get the div to work in all browsers. This is because bootstrap only targets the div in IE9 which is blah.
For now, we can just go with progress, using the div fallback within the progress so it should work with IE9. We can have the animated props which will add the class, but animations will not work until bootstrap fixes them. It probably the best route.

Edit
Well, with the tag prop, we can let the user choose, defaulting to progress. We can do some special logic for when the tag is progress or not; since they have different markup.

TheSharpieOne added a commit to TheSharpieOne/reactstrap that referenced this issue Aug 18, 2016
Add Progress component which will output bootstrap's progress component
Progress has `striped`, `animated`, and `color` props.
It will default to `<progress>` with a `<div>` fallback for IE9
Overriding the tag will produce the fallback inside the custom tag.

Closes reactstrap#78
TheSharpieOne added a commit to TheSharpieOne/reactstrap that referenced this issue Aug 18, 2016
Add Progress component which will output bootstrap's progress component
Progress has `striped`, `animated`, and `color` props.
It will default to `<progress>` with a `<div>` fallback for IE9
Overriding the tag will produce the fallback inside the custom tag.

Closes reactstrap#78
eddywashere pushed a commit that referenced this issue Aug 19, 2016
Add Progress component which will output bootstrap's progress component
Progress has `striped`, `animated`, and `color` props.
It will default to `<progress>` with a `<div>` fallback for IE9
Overriding the tag will produce the fallback inside the custom tag.

Closes #78
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 a pull request may close this issue.

2 participants