-
Notifications
You must be signed in to change notification settings - Fork 78
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
fix: eslint no unused vars #205
Changes from 1 commit
657231c
95c442e
be83198
b29a3c4
708e2b8
302b739
b59af2d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,7 +2,6 @@ import { mount } from 'enzyme'; | |
import React from 'react'; | ||
import renderer from 'react-test-renderer'; | ||
import { Shellbar } from './Shellbar'; | ||
import { Menu, MenuList, MenuItem } from '../Menu/Menu'; | ||
|
||
describe('<Shellbar />', () => { | ||
const profile1 = { | ||
|
@@ -51,101 +50,6 @@ describe('<Shellbar />', () => { | |
profileMenu={profileMenu} /> | ||
); | ||
|
||
const searchInput = { | ||
label: 'Search', | ||
placeholder: 'Enter a fruit', | ||
onSearch: function(searchTerm) { | ||
alert(`Search fired for ${searchTerm}`); | ||
}, | ||
callback: () => alert('Search selected!') | ||
}; | ||
|
||
const actions = [ | ||
{ | ||
glyph: 'settings', | ||
label: 'Settings', | ||
notificationCount: 5, | ||
callback: () => alert('Settings selected!'), | ||
menu: ( | ||
<Menu> | ||
<MenuList> | ||
<MenuItem url='/'>Option 1</MenuItem> | ||
<MenuItem url='/'>Option 2</MenuItem> | ||
<MenuItem url='/'>Option 3</MenuItem> | ||
</MenuList> | ||
</Menu> | ||
) | ||
} | ||
]; | ||
|
||
const notifications = { | ||
notificationCount: 2, | ||
label: 'Notifications', | ||
notificationsBody: ( | ||
<Menu> | ||
<MenuList> | ||
<MenuItem url='/'>Notification 1</MenuItem> | ||
<MenuItem url='/'>Notification 2</MenuItem> | ||
<MenuItem url='/'>Notification 3</MenuItem> | ||
</MenuList> | ||
</Menu> | ||
), | ||
noNotificationsBody: ( | ||
<Menu> | ||
<MenuList> | ||
<MenuItem>There are no notifications</MenuItem> | ||
</MenuList> | ||
</Menu> | ||
), | ||
callback: () => alert('Notification selected!') | ||
}; | ||
|
||
const profile = { | ||
initials: 'JS', | ||
userName: 'John Snow', | ||
colorAccent: 8 | ||
}; | ||
|
||
const productMenu = [ | ||
{ name: 'Application A', callback: () => alert('Application A selected!') }, | ||
{ name: 'Application B', callback: () => alert('Application B selected!') }, | ||
{ name: 'Application C', callback: () => alert('Application C selected!') }, | ||
{ name: 'Application D', callback: () => alert('Application D selected!') } | ||
]; | ||
|
||
const productSwitcherList = [ | ||
{ | ||
title: 'Fiori Home', | ||
glyph: 'home', | ||
callback: () => alert('Fiori Home selected!') | ||
}, | ||
{ | ||
title: 'S/4 HANA Cloud', | ||
glyph: 'cloud', | ||
callback: () => alert('S/4 HANA Cloud selected!') | ||
} | ||
]; | ||
|
||
const productSwitcher = { | ||
label: 'Product Switcher' | ||
}; | ||
|
||
const coPilotShell = ( | ||
<Shellbar | ||
logoSAP | ||
productTitle='Corporate Portal' | ||
productMenu={productMenu} | ||
subtitle='Subtitle' | ||
copilot | ||
searchInput={searchInput} | ||
actions={actions} | ||
notifications={notifications} | ||
profile={profile} | ||
profileMenu={profileMenu} | ||
productSwitcher={productSwitcher} | ||
productSwitcherList={productSwitcherList} /> | ||
); | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Were these made with the intention of using them in the snapshot tests? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's a good question. There were a bunch of variables defined to create it, but then it was never used. I will check the blame on this to see if someone wants to retain all this code in a separate branch, but it should all be removed from There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My bad, i think i forgot to make the snapshot call. I will create a chore: PR to add the call in there. This snapshot call bumps up the code coverage from 30% to about 85% There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @chrismanciero Do you want to fix it in this PR or create a separate one? |
||
test('create shellbar', () => { | ||
let component = renderer.create(simpleShellBar); | ||
let tree = component.toJSON(); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@greg-a-smith: why are these test being
x
-ed?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a to-do comment stating why snapshots were failing and the assertion was commented out causing the
no-unused-vars
lint error. To resolve all of the lint errors, the entire test ended up being commented out so I decided to uncomment everything and justx
the tests until the to-do comments are resolved.