Skip to content

Conversation

@arraydude
Copy link
Contributor

Issue: This PR fixes a bug when we try to stop recording before the countdown ends

Description:

  • Fixed bug when stop recording before countdown ends
  • Moved countdown logics to a new pure component
  • Cleaning countdown interval on unmount
  • Adding new 'Cancel' state to the menu
  • Unit tests
  • Update menu snapshot

Contribution License Agreement

By submiting this pull request you agree that all contributions to this project are made under the Apache 2.0 license.

… refactor/countdown

# Conflicts:
#	frontend/cypress/snapshots/linux/2x/st_main_menu.spec.ts/main_menu.snap.png
#	frontend/src/components/core/MainMenu/MainMenu.tsx
#	frontend/src/hocs/withScreencast/withScreencast.scss
#	frontend/src/hocs/withScreencast/withScreencast.tsx
@arraydude arraydude requested a review from a team as a code owner February 10, 2020 22:39
@arraydude arraydude requested a review from tvst February 11, 2020 15:36
/* The menu itself. */
div.dropdown-menu {
width: 12rem;
width: 13rem;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why change this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because the new "Cancel screencast" option needs a little bit more space

let outputBlob
const { currentState } = this.state

if (currentState === "OFF") {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this needed? Is it to avoid line 128 to show an error when state is "OFF"?

If so, just move the OFF check inside the if from line 126.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If the user press stop recording when the screencast is off , we should do nothing otherwise we we'll throw an exception to the frontend

import "./Countdown.scss"

interface Props {
countdown: number
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this a prop? We always count from 3 so this should just be hard-coded.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because we could use this countdown in a future for a different purpose.

@arraydude arraydude merged commit ab31e79 into streamlit:develop Feb 18, 2020
@arraydude arraydude deleted the refactor/countdown branch February 18, 2020 20:50
tconkling added a commit to tconkling/streamlit that referenced this pull request Feb 19, 2020
* develop: (30 commits)
  [Screencast] Fixing countdown bug (streamlit#1082)
  Release 0.56.0 (streamlit#1101)
  Fix bug where icon assets were missing from "make install" (streamlit#1100)
  Clean signatures of wrapped DeltaGenerator methods (streamlit#1099)
  Using widgetId as the key when rendering elements (streamlit#1102)
  Pass --skip-lock to pipenv under pipenv-install Makefile target. (streamlit#1093)
  Allow wider range of int-like types in NumberInput (streamlit#1087)
  Fix missing caching URL (streamlit#1094)
  Version 0.55.1 (just pointing to new docs) (streamlit#1089)
  Retire python-3.7.4 CircleCI project. (streamlit#1086)
  Handle different Caching error messages (streamlit#1068)
  Fix an incomplete copy for a dict that's then modified. (streamlit#1078)
  Fix indentation error in fad994c. (streamlit#1084)
  Widgets unit tests (streamlit#1077)
  [mypy] First phase of work on testing Python type annotations. (streamlit#1079)
  Add CircleCI jobs to run the oldest and newest Python versions we support. (streamlit#1053)
  Improve docs (streamlit#1075)
  Adding CORS to troubleshooting guide. (streamlit#1001)
  Fix for the CORS check (streamlit#965)
  Advanced caching updates feb (streamlit#1040)
  ...
tconkling added a commit to tconkling/streamlit that referenced this pull request Feb 19, 2020
# By Nahuel Emiliano Rosso Fandiño (1) and others
# Via GitHub
* develop:
  Show warning to user and use skipkeys=True if json.dumps causes TypeError (streamlit#1112)
  Server: store SessionInfo by id (streamlit#1056)
  [Screencast] Fixing countdown bug (streamlit#1082)

# Conflicts:
#	lib/streamlit/server/Server.py
tconkling added a commit to tconkling/streamlit that referenced this pull request Feb 24, 2020
* develop: (53 commits)
  Update plotly.js to 1.52 (streamlit#1119)
  Add tooltip in Altair/Vega-lite docstring code (streamlit#1092)
  Ability to bind to a server address with server.address config option (streamlit#1107)
  [mypy] Enable check_untyped_defs throughout the codebase. (streamlit#1110)
  st.map: set "radiusMinPixels" to 3 (streamlit#1113)
  st.plotly_chart docs incorrectly refer to Altair (streamlit#1118)
  Fix BytesIO and numpy array data sources for audio/video (streamlit#1116)
  Show warning to user and use skipkeys=True if json.dumps causes TypeError (streamlit#1112)
  Server: store SessionInfo by id (streamlit#1056)
  [Screencast] Fixing countdown bug (streamlit#1082)
  Release 0.56.0 (streamlit#1101)
  Fix bug where icon assets were missing from "make install" (streamlit#1100)
  Clean signatures of wrapped DeltaGenerator methods (streamlit#1099)
  Using widgetId as the key when rendering elements (streamlit#1102)
  Pass --skip-lock to pipenv under pipenv-install Makefile target. (streamlit#1093)
  Allow wider range of int-like types in NumberInput (streamlit#1087)
  Fix missing caching URL (streamlit#1094)
  Version 0.55.1 (just pointing to new docs) (streamlit#1089)
  Retire python-3.7.4 CircleCI project. (streamlit#1086)
  Handle different Caching error messages (streamlit#1068)
  ...
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