Skip to content
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

[KED-2982] Re-write Kedro Dropdown and Menu-option #721

Merged
merged 24 commits into from
Feb 15, 2022
Merged

[KED-2982] Re-write Kedro Dropdown and Menu-option #721

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
33f5d99
WIP
rashidakanchwala Jan 26, 2022
b3bcde5
Added menu option
rashidakanchwala Jan 26, 2022
ebed735
Fixed styling
rashidakanchwala Jan 26, 2022
156b147
Added tests
rashidakanchwala Jan 27, 2022
08beb3b
Merge branch 'main' into KED-2982/rewrite-kedro-dropdown-menu-options
rashidakanchwala Jan 28, 2022
71be293
Update release docs
rashidakanchwala Jan 28, 2022
c273b92
Remove unwanted code
rashidakanchwala Jan 28, 2022
cebf30c
WIP - classes to hook
rashidakanchwala Feb 8, 2022
ddc4664
refactored dropdown.js to use hooks
studioswong Feb 10, 2022
5471eab
removed unneccessary useRef
studioswong Feb 10, 2022
38cbd36
Added up/down arrow
rashidakanchwala Feb 10, 2022
4fba666
Merge branch 'KED-2982/rewrite-kedro-dropdown-menu-options' of github…
rashidakanchwala Feb 10, 2022
8bdf9e6
style alignments
tynandebold Feb 10, 2022
24ff11a
fix console error
tynandebold Feb 10, 2022
525fb71
cleanup
tynandebold Feb 10, 2022
76a90fe
keyboard events
tynandebold Feb 10, 2022
aec0bd6
Merge branch 'main' of https://github.com/kedro-org/kedro-viz into KE…
tynandebold Feb 10, 2022
1903616
update release notes
tynandebold Feb 10, 2022
883d76c
update release notes again
tynandebold Feb 10, 2022
27c5c68
Merge branch 'main' into KED-2982/rewrite-kedro-dropdown-menu-options
rashidakanchwala Feb 14, 2022
3314533
Merge branch 'main' into KED-2982/rewrite-kedro-dropdown-menu-options
rashidakanchwala Feb 14, 2022
4097d2d
further cleanup of comments
studioswong Feb 15, 2022
8ad3511
removed unneeded function
studioswong Feb 15, 2022
22f934a
Merge branch 'main' into KED-2982/rewrite-kedro-dropdown-menu-options
studioswong Feb 15, 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
12 changes: 5 additions & 7 deletions src/components/dropdown/dropdown.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ const Dropdown = (props) => {
}, [focusedOption]);

/**
* Handler for closing a dropdown if a click occured outside the dropdown.
* Handler for closing a dropdown if a click occurred outside the dropdown.
* @param {object} e - event object
*/
const _handleBodyClicked = (e) => {
Expand Down Expand Up @@ -212,7 +212,7 @@ const Dropdown = (props) => {
return previous;
};

return React.Children.toArray(this.props.children).reduce(
return React.Children.toArray(props.children).reduce(
getSectionChildren,
[]
);
Expand Down Expand Up @@ -288,11 +288,9 @@ const Dropdown = (props) => {
const _handleClose = () => {
setOpen(false);

this.setState({ open: false }, () => {
if (typeof onClosed === 'function') {
onClosed();
}
});
if (typeof onClosed === 'function') {
onClosed();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a callback on setting the state that needs to be handled in a useEffect to ensure this is only executed after the state change (i.e the setOpen state change here).

Regardless, on checking the code, this is actually already set up in the useEffects above, hence can be deleted.


// remove event listener
EventController.removeBodyListeners();
Expand Down
1 change: 1 addition & 0 deletions src/utils/key-events.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// Keyboard character codes
const KEYS = {
13: 'enter',
27: 'escape',
38: 'up',
40: 'down',
Expand Down