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

Jump to bottom

Migrate TimeSlider component to React #2231

Merged
merged 6 commits into from Mar 3, 2020
Merged

Migrate TimeSlider component to React #2231

merged 6 commits into from Mar 3, 2020

Conversation

fbarl
Copy link
Contributor

@fbarl fbarl commented Feb 21, 2020 •
edited

Part of #1681 (comment).

image

Changes

  • Generalized TimeAxis to accept transform prop and reused it for TimeSlider
  • Updated the way ticks are generated in TimeAxis to:
    • Include both ends of the time scale interval
    • Always have a fixed number of ticks
    • Render timestamps as Now if they are sufficiently close to the current time (within 15 minutes)
  • Modified some TimeSlider CSS styles to make the component more similar between my Chrome and Firefox
  • Accept startTime and endTime props in TimeSlider component (towards addressing Time slider doesn't account for gaps in data #2253)
  • Made all AreaGraphs show their overflown content (to render axis ticks)

Tested

  • The component looks consistent between my 64-bit Ubuntu Chrome (v79.0.3945.117) and Firefox (v71.0)
  • Interaction with area graphs works fine
  • Renders as Now for current time
  • Renders and acts well when no data (Germany)
  • Renders and interacts well on mobile
  • Responsive resizing

@fbarl fbarl self-assigned this Feb 21, 2020
@fbarl fbarl force-pushed the fbarl/2163-migrate-time-slider branch from 1a05af3 to 9c25bd5 Compare February 26, 2020 17:22
@fbarl fbarl mentioned this pull request Feb 26, 2020
Closed
@fbarl fbarl force-pushed the fbarl/2163-migrate-time-slider branch 4 times, most recently from 7d263ad to 05f8ca4 Compare March 3, 2020 12:13
@fbarl fbarl changed the title [WIP] Migrate TimeSlider component to React Migrate TimeSlider component to React Mar 3, 2020
@fbarl fbarl marked this pull request as ready for review March 3, 2020 12:24
@fbarl fbarl force-pushed the fbarl/2163-migrate-time-slider branch from 05f8ca4 to 47f526a Compare March 3, 2020 12:25
@fbarl fbarl requested a review from corradio March 3, 2020 12:25
corradio
corradio approved these changes Mar 3, 2020
View reviewed changes
Copy link
Member

@corradio corradio left a comment

Choose a reason for hiding this comment

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

looking good!

@fbarl fbarl merged commit ccca495 into master Mar 3, 2020
@fbarl fbarl deleted the fbarl/2163-migrate-time-slider branch March 3, 2020 14:04
@pierresegonne pierresegonne mentioned this pull request Jul 2, 2020
Closed
con-cat pushed a commit to con-cat/electricitymap-contrib that referenced this pull request May 18, 2021
@fbarl @con-cat
Migrate TimeSlider component to React (electricitymaps#2231)
8904eda 
* Migrate TimeSlider component to React

* Make TimeSlider a bit more robust

* Reuse TimeAxis component for TimeSlider

* Include current time as endTime for TimeSlider

* Made slider styles more responsive and consistent between Chrome and Firefox

* Small adjustments and fixes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@corradio corradio corradio approved these changes

Assignees

@fbarl fbarl

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@fbarl @corradio