feat: #560 google map web component #622
Conversation
auto-assign
bot
requested review from
dannd4,
nphivu414,
phmngocnghia,
duong-se,
vuhuucuong and
willmcvay
trankhacvy
force-pushed
the
feat/560-Google-Map-web-component
branch
from
7ba9ffc to
d0da1d3
Compare
trankhacvy
force-pushed
the
feat/560-Google-Map-web-component
branch
from
d0da1d3 to
9a940e9
Compare
| functions: 90, | ||
| lines: 98, | ||
| statements: 97, | ||
| functions: 70, |
There was a problem hiding this comment.
That's a big drop in test coverage - we should work on this
| Go: 4, | ||
| }, | ||
| BicyclingLayer: jest.fn().mockImplementation(function() { | ||
| // @ts-ignore |
There was a problem hiding this comment.
Why all the ts-ignores - surely the mock should conform to the type?
| price: string | ||
| } | ||
|
|
||
| export const getContent = ({ |
There was a problem hiding this comment.
Does this not make sense as a svelte component? I get why you wouldn't when it was a React component but Svelte is just HTML and CSS - we could get rid of the inline styles too
| @@ -0,0 +1,34 @@ | |||
| import svelte from 'rollup-plugin-svelte' | |||
There was a problem hiding this comment.
Don't think this file is needed anymore
| import GoogleMap from '../google-map.svelte' | ||
| import { render } from '@testing-library/svelte' | ||
| import { get } from 'svelte/store' | ||
| // import { loader } from '../../../../common/utils/loader' |
| let mapLoading: boolean = false | ||
|
|
||
| const unsubscribeSearchWidgetStore = searchWidgetStore.subscribe(store => { | ||
| if (map && storeInstance && storeInstance.properties !== store.properties) { |
There was a problem hiding this comment.
This is a note to me - I am not sure this is the best solution but will try and come up with something better - leave for now!
| createMockFuncsFromArray(instance, ['bindTo', 'get', 'notify', 'set', 'setValues', 'unbind', 'unbindAll']) | ||
| } | ||
|
|
||
| const maps = { |
There was a problem hiding this comment.
This mock gets loaded in the jest-setup file at the root of the project - might be not necessary at all!
| searchType?: SearchType | ||
| searchKeyword?: string | ||
| markers: google.maps.Marker[] | ||
| mapLoading: boolean |
There was a problem hiding this comment.
I think mapLoading is not now referenced in the code, if not, we can remove
| expect(createMarker).toHaveBeenCalledTimes(1) | ||
| }) | ||
|
|
||
| // it('should return correctly when no properties', () => { |
There was a problem hiding this comment.
See my comment about tests - we should test this function better - it is quite complex - recommend as a start point breaking down into blocks and testing them separately. Happy to help with this
| import { SearchWidgetStore } from '../core/store' | ||
| import { Writable } from 'svelte/store' | ||
|
|
||
| export const showPropertiesMarker = ( |
There was a problem hiding this comment.
See my previous - it's quite complex!
duong-se
changed the title
duong-se
changed the title
willmcvay
force-pushed
the
feat/560-Google-Map-web-component
branch
from
2770f45 to
09deee5
Compare
willmcvay
force-pushed
the
feat/560-Google-Map-web-component
branch
from
c642ef4 to
5cff980
Compare
github-actions
bot
added
aml-checklist
No description provided.