feat: add Map-block #149

Lakate · 2023-02-14T09:02:53Z

No description provided.

gravity-ui-bot · 2023-02-14T09:05:28Z

Preview is ready.

src/blocks/Map/__stories__/data.json

src/components/Map/Map.scss

src/components/Map/__stories__/ApiKeyInput.tsx

src/components/Map/__stories__/data.json

gorgeousvlad · 2023-02-15T13:41:46Z

src/blocks/Map/Map.tsx

+import Map from '../../components/Map/Map';
+import MediaBase from '../../components/MediaBase/MediaBase';
+
+export const MapBlock = (props: MapBlockProps) => {


export const MapBlock = ({map, ...props}: MapBlockProps) => {?

gorgeousvlad · 2023-02-15T13:45:42Z

src/blocks/Map/schema.ts

+                type: 'string',
+                enum: mediaDirection,
+            },
+            mobileDirection: {


could we take this props from media from its schema?

gorgeousvlad · 2023-02-15T13:49:10Z

src/context/mapsContext/mapsValueContext.ts

+};
+
+export const MapsContextSourceLinks = {
+    [MapType.Yandex]: 'https://api-maps.yandex.ru/2.1',


are sure that this yandex api host wouldn't have its place in production bundle? need to check

gorgeousvlad · 2023-02-15T13:51:08Z

src/context/mapsContext/mapsProvider.tsx

+
+export const apiKeyIdInLS = {
+    [MapType.Google]: 'gmap-api-key',
+    [MapType.Yandex]: '',


maybe we should not put this key at all instead of empty value?

gorgeousvlad · 2023-02-15T13:53:12Z

src/components/Map/GoogleMap.tsx

+            loading="lazy"
+            allowFullScreen
+            referrerPolicy="no-referrer-when-downgrade"
+            src={`${scriptSrc}?key=${apiKey}&q=${address.replace(' ', '+')}`}


maybe define func getScriptSrc?

gorgeousvlad · 2023-02-15T14:21:39Z

src/components/Map/YandexMapHelpers.ts

+    private recalcZoomAndCenter() {
+        const coordsLength = this.coords.length;
+
+        if (!coordsLength) return;


we try not to use one string ifs

gorgeousvlad · 2023-02-15T14:22:11Z

src/components/Map/YandexMapHelpers.ts

+            rightTop = [Math.max(rightTop[0], point[0]), Math.max(rightTop[1], point[1])];
+        });
+
+        // ~30px - control button height


create a constant please

gorgeousvlad · 2023-02-15T14:29:21Z

src/components/MediaBase/MediaBase.tsx

+                        {mediaContent}
+                    </Col>
+                    <Col sizes={mediaSizes}>
+                        <div className={b('card', {shadow: !disableShadow})}>{card}</div>


card could be null should we render the Col in this case?

gorgeousvlad · 2023-02-15T14:30:23Z

src/containers/PageConstructor/Provider.tsx

        <LocaleContext.Provider value={locale} />,
        <LocationContext.Provider value={location} />,
        <MobileContext.Provider value={Boolean(isMobile)} />,
+        <MapsValueContext.Provider value={mapsContext} />,


why do we need two contexts for maps, not one?

gorgeousvlad · 2023-02-15T14:32:29Z

src/context/mapsContext/mapsProvider.tsx

+    extends MapProviderExternalProps,
+        Partial<MapProviderDefaultProps> {}
+
+export class MapProvider extends React.Component<


we are trying to use only functional react components

src/blocks/Map/README.md

src/blocks/Map/schema.ts

src/components/Map/YandexMap.tsx

src/components/Map/YandexMapHelpers.ts

src/components/Map/__stories__/ApiKeyInput.tsx

src/components/Map/__stories__/Map.stories.tsx

src/context/mapsContext/mapsProvider.tsx

yuberdysheva · 2023-02-15T15:59:22Z

Is storybook maps don't rerender height between tabs. Only when we reload page

Lakate · 2023-02-21T13:51:46Z

I have changed maps:

you have to pass scriptSrc to the mapsContext
i created a helper file in storybook to store maps constants to use it in the stories only
After all, i don't see any constants connected with yandex maps api in build. (the YandexMap component remained in the builded files)

gorgeousvlad · 2023-02-22T06:50:07Z

src/blocks/Map/Map.tsx

+import Map from '../../components/Map/Map';
+import MediaBase from '../../components/MediaBase/MediaBase';
+
+export const MapBlock = ({map, ...props}: MapBlockProps) => {


not necessary but we could remove return here MapBlock = ({map, ...props}: MapBlockProps)=>();

gorgeousvlad · 2023-02-22T06:59:49Z

src/components/Map/YMap/YandexMapApiLoader.ts

+                YMapsApiLoader.status = MapApiStatus.Loaded;
+            })
+            .catch(() => {
+                throw new Error("Can't load Yandex Maps Api");


we try not to throw unhandled exceptions in ui, could you provide error situation in interface and display error for this case?

yuberdysheva · 2023-02-27T07:39:16Z

src/components/Map/Map.scss

+
+#{$block} {
+    width: 100%;
+    height: 300px;


I see that max-height below is also 300px, maybe we need a constant here?

Lakate requested review from gorgeousvlad and yuberdysheva as code owners February 14, 2023 09:02

Lakate requested review from DaffPunks and removed request for gorgeousvlad February 14, 2023 09:03

Lakate force-pushed the lakate/map-block branch from bdaad72 to f611e03 Compare February 14, 2023 09:22

yuberdysheva requested a review from gorgeousvlad February 14, 2023 09:33

aeksandla requested changes Feb 15, 2023

View reviewed changes

gorgeousvlad suggested changes Feb 15, 2023

View reviewed changes

yuberdysheva suggested changes Feb 15, 2023

View reviewed changes

Lakate force-pushed the lakate/map-block branch 3 times, most recently from 8f8e55e to f449d57 Compare February 21, 2023 13:51

gorgeousvlad reviewed Feb 22, 2023

View reviewed changes

Lakate force-pushed the lakate/map-block branch from 7c98557 to c33a807 Compare February 22, 2023 12:01

gorgeousvlad approved these changes Feb 22, 2023

View reviewed changes

yuberdysheva reviewed Feb 27, 2023

View reviewed changes

yuberdysheva approved these changes Feb 27, 2023

View reviewed changes

aeksandla approved these changes Feb 27, 2023

View reviewed changes

feat: add Map-block

13874f7

Lakate force-pushed the lakate/map-block branch from f753822 to 13874f7 Compare February 28, 2023 13:12

Lakate merged commit bd349bb into main Feb 28, 2023

Lakate deleted the lakate/map-block branch February 28, 2023 13:15

feat: add Map-block #149

feat: add Map-block #149

Uh oh!

Conversation

Lakate commented Feb 14, 2023

Uh oh!

gravity-ui-bot commented Feb 14, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

yuberdysheva commented Feb 15, 2023

Uh oh!

Lakate commented Feb 21, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Lakate commented Feb 21, 2023 •

edited

Loading