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
[CORL-1334] Keyboard shortcuts #3158
Conversation
src/core/client/stream/common/KeyboardShortcuts/KeyboardShortcuts.tsx
Outdated
Show resolved
Hide resolved
7d325cf
to
1f43ab9
Compare
3e22d70
to
63067b3
Compare
src/core/client/embed/PymControl.ts
Outdated
@@ -15,7 +15,7 @@ export const defaultPymControlFactory: PymControlFactory = (config) => | |||
new PymControl(config); | |||
|
|||
export default class PymControl { | |||
private pym: pym.Parent; | |||
public pym: pym.Parent; |
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.
Revert to private
as it doesn't seem it's used elsewhere.
src/core/client/embed/StreamEmbed.ts
Outdated
@@ -167,6 +168,8 @@ export class StreamEmbed { | |||
decorators: streamDecorators, | |||
url, | |||
}); | |||
|
|||
hookUpWindowEvents(this.pymControl); |
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.
Use the existing streamDecorators
array above and rewrite the hookUpWindowEvents
as a decorator so it supports a unsubscribe function.
See https://github.com/coralproject/talk/blob/master/src/core/client/embed/decorators/withClickEvent.ts for an example.
}, []); | ||
|
||
const toKeyStop = useCallback((el: Element) => { | ||
const id = el.attributes.getNamedItem("data-keyid"); |
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.
Data attributes can be retrieved instead via:
el.dataset.keyid
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.
Note that you must use HTMLElement
instead of Element
here to allow this.
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.
Note that if you rewrite access for data attributes to use kebab case, access via .dataset
is done via camel case:
el.dataset.isLoadMore
@@ -264,6 +266,8 @@ export const AllCommentsTabContainer: FunctionComponent<Props> = ({ | |||
disabled={isLoadingMore} | |||
aria-controls="comments-allComments-log" | |||
className={CLASSES.allCommentsTabPane.loadMoreButton} | |||
data-keystop={true} | |||
data-isLoadMore={true} |
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.
Data props typically are written in kebab case:
data-key-stop
data-is-load-more
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 don't think that there is a convention.
aria
props does not separate words:
aria-labelledby
aria-describedby
either way is fine for me though.
CORL-1334
Will get ready for using relay as our comment tree. CORL-1334
- Still need to replace X key with Shift+C to go backwards when walking through comments CORL-1334
CORL-1334
CORL-1334
CORL-1334
63067b3
to
9ec30b9
Compare
What does this PR do?
Allows you to use the
C
andShift + C
key to traverse through the comment stream.Changes to schema?
None
How to test
C
to traverse through visible comment treeShift + C
to reverse traverse through comments