Add icons for play/pause/sync actions #944
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
robertknight
commented
May 30, 2023
There was a problem hiding this comment.
This file was reformatted by Prettier, but is otherwise unchanged.
|
There are some lint errors I need to resolve: Edit: Resolved by copying a lint override from the hypothesis/frontend-shared repo. |
robertknight
force-pushed
the
play-pause-sync-icons
branch
from
d561174 to
b307139
Compare
- Import SVG icons for Play, Pause and Sync actions from the interactive mock at https://hypothesis-video-transcriptions.netlify.app - Use Prettier to format SVG files, treating them as HTML
These were generated using the `generate-icons.js` script from the hypothesis/frontend-shared repo using: ``` cd via/static/ node ~/hypothesis/frontend-shared/scripts/generate-icons.js images/icons/ scripts/video_player/components/icons/ ``` Where `~/hypothesis/frontend-shared` is the location where the `hypothesis/frontend-shared` repo is checked out.
This was copied from src/components in the @hypothesis/frontend-shared project.
Replace emojis with SVG icons taken from the UI prototype at https://hypothesis-video-transcriptions.netlify.app. Also adjust the layout of the top bar to more closely match that prototype.
robertknight
force-pushed
the
play-pause-sync-icons
branch
from
b307139 to
03189ce
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Replace the placeholder emoji "icons" for play/pause actions with proper SVG icons, and add an icon for the sync button.
The SVG files were taken from the UI prototype at https://hypothesis-video-transcriptions.netlify.app and converted to Preact components using the
generate-icons.jsscript in the hypothesis/frontend-shared repo (see also hypothesis/frontend-shared#1064).The layout of the top bar has also been modified to more closely match the prototype. I'm not sure that we'll want to have the search bar always be visible and whether it should be above/below the buttons, but it is useful for it to have its own row where it constrained for horizontal space by the buttons around it.
Before:
(nb. Appearance of play/pause button varied by platform)
After (Paused state):
After (Playing state):