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.

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

GC-1083 - Fix console log spam in item editor #1220

Merged
merged 4 commits into from
Jun 21, 2023

Conversation

timbryandev
Copy link
Contributor

💬 Description

Fixing some issues that are causing spammy console logs and actual issues potentially getting missed

#BrokenWindows

showContent: showContentProp,
toggleShowContent: toggleShowContentProp,
...restProps
} = props;
Copy link
Contributor

Choose a reason for hiding this comment

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

same q for the one below, why are we destructuring again here rather than in the main props destructuring above?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

in this case there was a naming collision when I tried to destructing in the function arguments directly with the destructuring that happens on the DropdownContext below.

I guess i could rename the values in the component props when destructuring like this but it didn't feel right somehow

...
  value,
  showContent: showContentProp,
  toggleShowContent: toggleShowContentProp,
  ...props
}) => {
  const {
    toggleShowContent = toggleShowContentProp,
    showContent = showContentProp
  } = useContext(DropdownContext) || {};
...

I guess my actual issue is refactoring this bit to prevent the collision:

  const {
    toggleShowContent = toggleShowContentProp,
    showContent = showContentProp
  } = useContext(DropdownContext) || {};

I'll have a rethink - think it was my late afternoon brain failing me on this one

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, the other issue was alongside the naming issue, we are also setting default values so that is the dropdown context returns nothing, we default to the values from the props - that tripped up my other attempts too

@timbryandev timbryandev merged commit 212194c into main Jun 21, 2023
2 checks passed
@timbryandev timbryandev deleted the debt/GC-1083_Fix_console_log_spam_in_item_editor branch June 21, 2023 13:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants