Skip to content

avoid using this.set (fixes #1275) #1276

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

Merged
merged 1 commit into from
Jul 4, 2022
Merged

avoid using this.set (fixes #1275) #1276

merged 1 commit into from
Jul 4, 2022

Conversation

geneukum
Copy link
Contributor

@geneukum geneukum commented Jul 2, 2022

Right now, the DocsKeyboardShortcutsComponent attempts to use this.set
to set a property. DocsKeyboardShortcutsComponent is a Glimmer
Component, so this API is not available. To avoid the problem, let's
use a regular assignment statement.

Right now, the DocsKeyboardShortcutsComponent attempts to use `this.set`
to set a property. DocsKeyboardShortcutsComponent is a Glimmer
Component, so this API is not available.  To avoid the problem, let's
use a regular assignment statement.
@@ -22,9 +22,9 @@ export default class DocsKeyboardShortcutsComponent extends Component {
@onKey('KeyG', { event: 'keyup' })
goto() {
if (!formElementHasFocus()) {
this.set('isGoingTo', true);
this.isGoingTo = true;
Copy link
Member

Choose a reason for hiding this comment

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

Does isGoingTo need to be a tracked prop?

Copy link
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 so.

My understanding is that isGoingTo is basically just an internal flag that we're using to chain together multiple keyup events for a keyboard shortcut rather than something that affects how the component is rendered.

@RobbieTheWagner RobbieTheWagner merged commit eadb33c into ember-learn:master Jul 4, 2022
@geneukum geneukum deleted the fix-docs-keyboard-shortcuts-typeerror branch July 4, 2022 15:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants