-
Notifications
You must be signed in to change notification settings - Fork 6
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
Add copy event id/hash button. #114
Open
Nufflee
wants to merge
5
commits into
tsoding:master
Choose a base branch
from
Nufflee:copy-event-id
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
7aa083e
Add event support to Tag.
Nufflee 5dd3274
Add a button to copy event id/hash.
Nufflee f81adc7
Use our own implementation of copyToClipboard.
Nufflee 9681e66
Add .vscode/ to .gitignore
Nufflee aa29eb7
Add timestamp() function to dto.Event.
Nufflee File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,4 @@ | ||
node_modules/ | ||
dist/ | ||
ts/**/*.js | ||
ts/**/*.js | ||
.vscode/ |
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
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
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
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
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
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
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
import * as dto from './dto'; | ||
import * as html from './html'; | ||
import * as moment from 'moment'; | ||
import ComponentsArray from './ComponentsArray'; | ||
import Countdown from './Countdown'; | ||
import UiComponent from './UiComponent'; | ||
import TwitchPlayer from './TwitchPlayer'; | ||
import copyToClipboard from './util/copyToClipboard'; | ||
|
||
export default class Timestamp implements UiComponent { | ||
constructor(private _timestamp: string) { | ||
} | ||
|
||
onCopyClick = () => { | ||
copyToClipboard(this._timestamp) | ||
} | ||
|
||
appendTo(entry: HTMLElement | null): void { | ||
new html.Div( | ||
new ComponentsArray([ | ||
new html.Href( | ||
`#_${this._timestamp}`, | ||
new html.Text(`${this._timestamp}`) | ||
), | ||
new html.Tag( | ||
"i", | ||
new html.Empty(), | ||
{"class": "copy fas fa-copy fa-lg"}, | ||
{"click": this.onCopyClick} | ||
) | ||
]), | ||
{"class": "timestamp"} | ||
).appendTo(entry); | ||
} | ||
} |
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,9 +1,14 @@ | ||
import * as moment from 'moment'; | ||
|
||
export default interface Event { | ||
datetime: moment.Moment, | ||
title: string, | ||
description: string, | ||
url: string, | ||
channel: string, | ||
export default class Event { | ||
constructor(public datetime: moment.Moment, | ||
public title: string, | ||
public description: string, | ||
public url: string, | ||
public channel: string) { | ||
} | ||
|
||
timestamp() { | ||
return this.datetime.utc().unix().toString(); | ||
} | ||
} |
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
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,49 @@ | ||
// Adapted from https://github.com/feross/clipboard-copy | ||
|
||
export default function copyToClipboard(text: string) { | ||
// Use the Async Clipboard API when available. Requires a secure browing context (i.e. HTTPS) | ||
Nufflee marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if ((navigator as any).clipboard) { | ||
return (navigator as any).clipboard.writeText(text) | ||
} | ||
|
||
// ...Otherwise, use document.execCommand() fallback | ||
|
||
// Put the text to copy into a <span> | ||
const span = document.createElement('span') | ||
span.textContent = text | ||
|
||
// Preserve consecutive spaces and newlines | ||
span.style.whiteSpace = 'pre' | ||
|
||
// Add the <span> to the page | ||
document.body.appendChild(span) | ||
|
||
// Make a selection object representing the range of text selected by the user | ||
const selection = window.getSelection() | ||
const range = window.document.createRange() | ||
|
||
if (!selection) { | ||
return Promise.reject() | ||
} | ||
|
||
selection.removeAllRanges() | ||
range.selectNode(span) | ||
selection.addRange(range) | ||
|
||
// Copy text to the clipboard | ||
let success = false | ||
try { | ||
success = window.document.execCommand('copy') | ||
} catch (err) { | ||
console.log('Failed to copy text. Error: ', err) | ||
} | ||
|
||
// Cleanup | ||
selection.removeAllRanges() | ||
window.document.body.removeChild(span) | ||
|
||
// The Async Clipboard API returns a promise that may reject with `undefined` so we match that here for consistency. | ||
return success | ||
? Promise.resolve() | ||
: Promise.reject() | ||
} |
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought I'd chime in since I was watching @Nufflee work on this on stream.
e as dto.Event
is an unchecked cast and it is an invalid cast in this particular case.e
is a plain javascript object that is defined in either of the returns on lines 41 or 57. Casting this object todto.Event
does not convert it into an instance of the class. However, this is an interesting situation because the application works correctly despite this bug. As passed to the constructor ofPatchedEvent
, the object instance is missing thetimestamp()
method and callinginstance of Event
on it would return false. What "fixes" this, is that thePatchedEvent
dto class inherits from theEvent
dto class. Thesuper
call uses the properties of the passed inevent
object to instantiate a correctEvent
dto object whichPatchedEvent
inherits from, so the object from the invalid cast is never used anywhere else. This just so happens to work because the property names in theEvent
dto class happily line up with the property names in the returns inEventsForDay.ts
(again, lines 41 and 57), but this is just "coincidence" - there is no type checking here.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see and I think I understand what you're talking about but what's the solution your suggesting? Actually creating a new instance instead of casting?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I may also add my opinion to this: by changing some of the dtos from interfaces to classes, the code is now written in two different paradigms: structural typing and OOP with prototype-based inheritance. Before, it was only structural typing, which I would argue is a better fit for the use case and there is no need to introduce OOP here, if patching events was handled like so:
However, obviously, this is not my codebase, so please treat this as a suggestion.