Remove some unused variables && properties #4843
Conversation
KKoukiou
force-pushed
the
autofix-some-eslint-warnings
branch
from
31ffd17
to
51d5f5b
Compare
KKoukiou
changed the title
|
PF4 preview: https://patternfly-react-pr-4843.surge.sh |
KKoukiou
force-pushed
the
autofix-some-eslint-warnings
branch
from
51d5f5b
to
f4c5c3f
Compare
There is not corresponding issue - I just found some problems while checking the codebase and fixed them. |
| @@ -12,8 +12,6 @@ export interface DropdownMenuProps { | |||
| className?: string; | |||
| /** Flag to indicate if menu is opened */ | |||
| isOpen?: boolean; | |||
| /** Flag to indicate if menu should be opened on enter */ | |||
| openedOnEnter?: boolean; | |||
There was a problem hiding this comment.
I checked patternfly-analytics and none of our known consumers are using this prop.
However, since we export this component in Dropdown/index.ts it's better to just mark it as deprecated and destructure it so that it's not passed to the DOM. If we remove it entirely we might break some Typescript consumer who is using it, or some Javascript consumer might be confused why their MenuComponent has openedOnEnter passed to it which propagates down to an HTMLElement.
There was a problem hiding this comment.
I see your point. It's sad that we have to keep a property which literally does nothing but you are completely right.
There was a problem hiding this comment.
@redallen btw this component is not exposed at all in the documentation website. https://www.patternfly.org/v4/documentation/react/components/dropdown
I would say then that since it's not there, it's properties change should not worry us.
KKoukiou
force-pushed
the
autofix-some-eslint-warnings
branch
from
f4c5c3f
to
0e7482c
Compare
KKoukiou
force-pushed
the
autofix-some-eslint-warnings
branch
from
0e7482c
to
2ed8008
Compare
| @@ -95,10 +95,9 @@ export class DropdownWithContext extends React.Component<DropdownProps & OUIAPro | |||
| <DropdownMenu | |||
| setMenuComponentRef={this.setMenuComponentRef} | |||
| component={component} | |||
| isOpen={isOpen} | |||
| isOpen={isOpen || openedOnEnter} | |||
There was a problem hiding this comment.
Probably better to just leave it off here
|
…ever used Mark it as deprecated and don't pass it when using this component because it's ignored anyway.
* componentContent variables in InternalDropdownItem component was reset after last use * unused 'styles' import in NotificationDrawerList component * ununsed 'ev' parameter in event handler of Page component
…le in jest test Fixes Warning: React does not recognize the `isFocused` prop on a DOM element.
KKoukiou
force-pushed
the
autofix-some-eslint-warnings
branch
from
2ed8008
to
06fc473
Compare
@tlabaj in patternfly do you really create an issue for each PR? Especially for this case right here creating an issue does not really make sense IMO, because it's multiple unrelated between then fixes. It's mostly a repository maintenance PR and not targeting specific issue. Opening an issue for such PRs will just create unnecessary noise. Or am I missing something? |
|
@KKoukiou Yes, this is the outlined here in our contribution guidelines. |
…le in jest test Fixes Warning: React does not recognize the `isHovered` prop on a DOM element. Closes patternfly#4874
KKoukiou
force-pushed
the
autofix-some-eslint-warnings
branch
from
06fc473
to
8a93ce9
Compare
Thanks for the answer :) I created an issue added the fixes line and reposted. |
No description provided.