stephane-monnot / react-vertical-timeline

Vertical timeline for React.js
https://stephane-monnot.github.io/react-vertical-timeline/
MIT License
1.07k stars 158 forks source link

Feature/hide empty content #11

Closed joeyparis closed 6 years ago

joeyparis commented 6 years ago

This hides the floating white box if no children are passed into the VerticalTimelineElement allowing for the timeline to have entries that are just icons or just icons and dates. This probably isn't the most effective way to do this but was the least disruptive to the existing code.

I understand if this becomes an "unwanted behavior" and isn't accepted but I personally found it useful in my use case.

codecov-io commented 6 years ago

Codecov Report

Merging #11 into master will increase coverage by 0.22%. The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #11      +/-   ##
==========================================
+ Coverage   96.55%   96.77%   +0.22%     
==========================================
  Files           2        2              
  Lines          29       31       +2     
  Branches        6        7       +1     
==========================================
+ Hits           28       30       +2     
  Misses          1        1
Impacted Files Coverage Δ
src/VerticalTimelineElement.jsx 95.45% <100%> (+0.45%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 2a4d953...a19b511. Read the comment docs.

stephane-monnot commented 6 years ago

My suggestions :

joeyparis commented 6 years ago

Thanks for the suggestions! I have adjusted the CSS classes to match your suggestions.

I am new to testing so I apologize in advance if any of my tests don't follow proper testing conventions.

I also added an example to the demo page, but of course feel free to adjust that example in whatever way you feel is best for the demo.

This should be good to merge now. It is passing all testing including lint.

stephane-monnot commented 6 years ago

Good job. Thanks. I will check demo and bump version. It's an old way to test, so I will replace with Enzyme when I have time.

stephane-monnot commented 6 years ago

Available on 2.1.1.

joeyparis commented 6 years ago

No, thank you! I'm working on a pretty big feature for my project that's all sort of centered around a "timeline" aspect so I'll probably be adding a small handful of features to your timeline component if that's okay with you, otherwise I can just fork my own version.

I'm very new to effectively programming tests but my allocated time for this stretch of my project is pretty large and there are relatively few (and also simple) tests so I can probably fit that transition into my development schedule over the next ~2 weeks.

stephane-monnot commented 6 years ago

I'm glad to hear that. I will appreciate your contributions.