Skip to content

Fix/pause at end of file#57

Merged
meganrm merged 4 commits intomasterfrom
fix/pause-at-end-of-file
Oct 21, 2020
Merged

Fix/pause at end of file#57
meganrm merged 4 commits intomasterfrom
fix/pause-at-end-of-file

Conversation

@meganrm
Copy link
Copy Markdown
Contributor

@meganrm meganrm commented Oct 20, 2020

Problem: when the trajectory got to the end of the file, it stopped playing, but the play button was still in the "playing" state.
Fix:

  1. when the trajectory gets to the end of the data, the play button switches to the "paused" state.
  2. If a user hits the play button when the file has played to the end, it will start playing from the beginning.

Other fixes:

  1. Disabled buttons were getting a hover state
  2. Added formatting for seconds to the slider tooltip.

Pull request recommendations:

  • Name your pull request your-development-type/short-description. Ex: feature/read-tiff-files
  • Link to any relevant issue in the PR description. Ex: Resolves [gh-##], adds tiff file format support
  • Provide description and context of changes.
  • Provide relevant tests for your feature or bug fix.
  • Provide or update documentation for any feature added by your pull request.

Thanks for contributing!

@meganrm meganrm requested review from a user, blairlyons, schoinh and toloudis October 20, 2020 22:15
background-color: var(--dark-theme-btn-disabled-bg);
color: var(--dark-theme-btn-disabled-color);

background-color: var(--dark-theme-btn-disabled-bg)!important;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Is there a reason to use !important here over disabling pointer events?
not a major issue here, but afaik !important is often avoided since it can be difficult to debug in conflicting cases

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

antd had it own [disabled]:hover color state. I could do that instead but all of this is confined to just the buttons in this folder

Comment thread src/containers/ViewerPanel/index.tsx Outdated
simulariumController.playFromTime(time + timeStep);
const { time, timeStep, simulariumController, totalTime } = this.props;
let newTime = time + timeStep;
if (newTime === totalTime) {
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.

Is there any chance newTime could be greater than totalTime?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I don't think it ever should be, but I did use that as a check in other places in case there is a rounding issue somewhere or something, I cad add that here

@meganrm meganrm merged commit f58e07b into master Oct 21, 2020
@meganrm meganrm deleted the fix/pause-at-end-of-file branch December 14, 2020 18:29
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.

2 participants