Skip to content

Fix 0 Toggle panels shortcut not properly working #648

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.

Sign up for GitHub

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 7 commits into from
Dec 6, 2022
Merged

Fix 0 Toggle panels shortcut not properly working #648

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
a00448b
Fix keydown/keyup listeners for shortcuts not properly removed when e…
vincentfretin Oct 10, 2022
5a66bf9
there is no need to call forceUpdate after calling setState
vincentfretin Oct 10, 2022
1970e38
don't show + buttons when inspector is disabled
vincentfretin Oct 10, 2022
e7447f7
fix right panel + button to really be on the right
vincentfretin Oct 10, 2022
ebfbd89
toggling a panel shouldn't impact the other one
vincentfretin Oct 10, 2022
b394598
add top 0 on left and right classes so the + button really shows up
vincentfretin Oct 10, 2022
0a9fed5
replace spread syntax by Object.assign
vincentfretin Oct 11, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 10 additions & 8 deletions src/components/Main.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,18 +50,18 @@ export default class Main extends React.Component {
} else if (event.which === 'attributes') {
this.setState(prevState => ({
visible: {
...prevState.visible,
attributes: !prevState.visible.attributes
}
}));
} else if (event.which === 'scenegraph') {
this.setState(prevState => ({
visible: {
...prevState.visible,
scenegraph: !prevState.visible.scenegraph
}
}));
}

this.forceUpdate();
});
}

Expand Down Expand Up @@ -109,15 +109,18 @@ export default class Main extends React.Component {
};

renderComponentsToggle() {
if (!this.state.entity || this.state.visible.attributes) {
if (
!this.state.inspectorEnabled ||
!this.state.entity ||
this.state.visible.attributes
) {
return null;
}
return (
<div className="toggle-sidebar right">
<a
onClick={() => {
this.setState({ visible: { attributes: true } });
this.forceUpdate();
Events.emit('togglesidebar', { which: 'attributes' });
}}
className="fa fa-plus"
title="Show components"
Expand All @@ -127,15 +130,14 @@ export default class Main extends React.Component {
}

renderSceneGraphToggle() {
if (this.state.visible.scenegraph) {
if (!this.state.inspectorEnabled || this.state.visible.scenegraph) {
return null;
}
return (
<div className="toggle-sidebar left">
<a
onClick={() => {
this.setState({ visible: { scenegraph: true } });
this.forceUpdate();
Events.emit('togglesidebar', { which: 'scenegraph' });
}}
className="fa fa-plus"
title="Show scenegraph"
Expand Down
6 changes: 3 additions & 3 deletions src/lib/shortcuts.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,8 +173,8 @@ var Shortcuts = {
this.disable();
}

window.addEventListener('keydown', this.onKeyDown.bind(this), false);
window.addEventListener('keyup', this.onKeyUp.bind(this), false);
window.addEventListener('keydown', this.onKeyDown, false);
window.addEventListener('keyup', this.onKeyUp, false);
this.enabled = true;
},
disable: function() {
Expand All @@ -188,7 +188,7 @@ var Shortcuts = {
this.shortcuts.modules[moduleName][keyCode]
) {
console.warn(
'Keycode <%s> already registered as shorcut within the same module',
'Keycode <%s> already registered as shortcut within the same module',
keyCode
);
}
Expand Down
12 changes: 6 additions & 6 deletions src/style/index.styl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -247,12 +247,6 @@ body.aframe-inspector-opened
position absolute
z-index 9998

.left
left 0

.right
right 0

a
background-color #262626
color #bcbcbc
Expand All @@ -262,3 +256,9 @@ body.aframe-inspector-opened
a.hover
background-color #1faaf2
color #fff

.toggle-sidebar.left
left 0

.toggle-sidebar.right
right 0
Copy link
Contributor Author

@vincentfretin vincentfretin Oct 10, 2022

Choose a reason for hiding this comment

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

Previous rule matched element with "right" class as a child of element having "toggle-sidebar" class. This is not what we want here. The div has both the "toggle-sidebar" and "right" classes, so we want a ".toggle-sidebar.right" rule without space. If you put a space, it means the same as the previous rule.