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.

Sign up for GitHub

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

[map-layers] migrate map layers to itwinui 3 #1136

Draft
wants to merge 14 commits into
base: master
Choose a base branch
from
Draft

[map-layers] migrate map layers to itwinui 3 #1136

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
855e436
migrate map layers to itwinui 3, fix build errors
hl662 Jan 7, 2025
705d27b
Merge branch 'master' into nam/map-layer-ui3
hl662 Jan 7, 2025
8f944c5
update missing documentation, update deps
hl662 Jan 14, 2025
ae8ca29
one test done...
hl662 Jan 14, 2025
ef0dfed
just mapManagerSettings tests left
hl662 Jan 15, 2025
f91c7fb
Merge remote-tracking branch 'origin/master' into nam/map-layer-ui3
hl662 Jan 16, 2025
cc2f4ec
fix the remaining test
hl662 Jan 16, 2025
2fb5767
Empty commit
hl662 Jan 16, 2025
fa304dd
replace deprecated event.Keycode
hl662 Jan 16, 2025
4a9536a
update itwin eslint to 5.x, related deps to latest, removed unneeded …
hl662 Jan 16, 2025
1435cb4
update changefile to use minor
hl662 Jan 16, 2025
626d893
update changefile to be major, bump itwin core pkgs to latest, remove…
hl662 Jan 16, 2025
60f39c7
address imodel components react deprecations, bump up to react 18, ad…
hl662 Jan 17, 2025
42a83b6
remove pnpm override temporarily, add some comfortable refactored com…
hl662 Jan 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions change/@itwin-map-layers-ccc19b28-3b8c-4963-aa0f-a5d8a10efa3a.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
Copy link
Member

Choose a reason for hiding this comment

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

updating peers will technically be a major ver bump,
might be able to get away with minor here but that might piss off some,
if we do a major ver bump, id want to make sure we are going to the latest versions of 4.x and react to all deprecations

Copy link
Author

@hl662 hl662 Jan 16, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I changed this to minor, could be keen on doing major, after running the linter I've reacted to the deprecations in appui, no other deprecations were found (aside from event.Keycode)

Copy link
Member

Choose a reason for hiding this comment

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

id ant to update core and all other peer deps to the latest as well if we do a major bump

Copy link
Member

Choose a reason for hiding this comment

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

we could split that into multiple prs, just need to hold off on publishing map-layers pkg until we are ready, or we do pre-releases

Copy link
Author

@hl662 hl662 Jan 16, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I switched the changefile type to major and bumped all core pkgs and other peerDeps to the latest, minus appui-react and imodel-components-react (which would require bumping up to their 5.x vers). Do we want to bump those 2? Should we also update the peerDeps to the latest as well?

Copy link
Member

Choose a reason for hiding this comment

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

i would look at what that change means, im not sure how big of an update it will be for those pkgs

Copy link
Author

Choose a reason for hiding this comment

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

it's quite big... some of the components used from `imodel-components-react' are marked deprecated. On top of that, these 2 packages drop support for react 17, so we have to bump up to react 18, and respectively update peer deps of React. We'll have to use new components, edit the test cases and update type definitions for some of the react hooks in the package. Probably a larger amount of effort than what this PR is about

Copy link
Member

Choose a reason for hiding this comment

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

there are only 3 imports from imodel-components-react
i imagine the colorswatch components are deprecated in favor of itwinui v3 components which we should be using

Copy link
Member

Choose a reason for hiding this comment

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

also now that im looking i see there was a mixup on what latest meant, i meant the latest 4.x, not latest version of appui which is 5.0, my apologies, shouldve been more clear

Copy link
Member

Choose a reason for hiding this comment

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

instead of going directly to a major, we should publish a pre-release to make sure everything works, and can also look updating other peer dep majors to include in the full major release

"comment": "migrate map-layers to iTwinUI 3",
"packageName": "@itwin/map-layers",
"email": "[email protected]",
"dependentChangeType": "patch"
}
2 changes: 2 additions & 0 deletions packages/itwin/map-layers/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ This package can also be installed into an application and the method MapLayersU
2. The dependencies are installed as part of "rush install" in the iTwin.js repository.
3. Build the extension as part of the "rush build" in the iTwin.js repository, or separately build using the npm build command.

> For the base imagery to appear, you will need to include a `IMJS_BING_MAPS_KEY` to your environment variables.

```sh
npm run build
```
Expand Down
28 changes: 12 additions & 16 deletions packages/itwin/map-layers/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,28 +38,24 @@
},
"dependencies": {
"@itwin/itwinui-icons-color-react": "^2.1.0",
"@itwin/itwinui-icons-react": "^2.6.0",
"@itwin/itwinui-react": "^2.11.2",
"classnames": "^2.3.1",
"@itwin/itwinui-icons-react": "^2.8.0",
"react-beautiful-dnd": "^13.1.1"
},
"devDependencies": {
"@itwin/appui-abstract": "^4.3.0",
"@itwin/appui-layout-react": "^4.0.1",
"@itwin/appui-react": "^4.0.1",
"@itwin/build-tools": "^4.3.0",
"@itwin/components-react": "^4.0.1",
"@itwin/components-react": "^4.10.0",
"@itwin/core-bentley": "^4.3.0",
"@itwin/core-common": "^4.3.0",
"@itwin/core-frontend": "^4.3.0",
"@itwin/core-geometry": "^4.3.0",
"@itwin/core-orbitgt": "^4.3.0",
"@itwin/core-quantity": "^4.3.0",
"@itwin/core-react": "^4.0.1",
"@itwin/core-react": "^4.10.0",
"@itwin/core-telemetry": "^4.3.0",
"@itwin/eslint-plugin": "^4.0.0-dev.38",
"@itwin/imodel-components-react": "^4.0.1",
"@itwin/itwinui-variables": "^2.0.0",
"@itwin/eslint-plugin": "^5.0.0",
"@itwin/imodel-components-react": "^4.10.0",
"@itwin/itwinui-react": "^3.16.0",
"@itwin/map-layers-formats": "^4.3.0",
"@itwin/webgl-compatibility": "^4.3.0",
"@testing-library/dom": "^8.12.0",
Expand Down Expand Up @@ -105,18 +101,18 @@
"source-map-support": "^0.5.6",
"ts-node": "^10.9.1",
"typemoq": "^2.1.0",
"typescript": "~4.6.0"
"typescript": "~5.0.0"
},
"peerDependencies": {
"@itwin/appui-abstract": "^4.0.0",
Copy link
Member

Choose a reason for hiding this comment

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

maybe you should update the core peers to the latest?

"@itwin/appui-layout-react": "^4.0.0",
"@itwin/appui-react": "^4.0.0",
"@itwin/components-react": "^4.0.0",
"@itwin/appui-react": "^4.10.0",
"@itwin/components-react": "^4.10.0",
"@itwin/core-bentley": "^4.3.0",
"@itwin/core-common": "^4.3.0",
"@itwin/core-frontend": "^4.3.0",
"@itwin/core-react": "^4.3.0",
"@itwin/imodel-components-react": "^4.0.0",
"@itwin/core-react": "^4.10.0",
"@itwin/imodel-components-react": "^4.10.0",
"@itwin/itwinui-react": "^3.16.0",
"@itwin/map-layers-formats": "^4.3.0",
"react": "^17.0.0",
"react-dom": "^17.0.0"
Expand Down
2,010 changes: 1,552 additions & 458 deletions packages/itwin/map-layers/pnpm-lock.yaml
View file
Open in desktop

Large diffs are not rendered by default.

52 changes: 30 additions & 22 deletions packages/itwin/map-layers/src/test/BasemapPanel.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,17 @@

import { expect, should } from "chai";
import * as sinon from "sinon";
import * as coreCommon from "@itwin/core-common";
import * as coreFrontend from "@itwin/core-frontend";
import { fireEvent, render } from "@testing-library/react";
import {
BackgroundMapProvider,
BackgroundMapType,
BaseLayerSettings,
BaseMapLayerSettings,
ColorByName,
ColorDef,
MapImagerySettings,
} from "@itwin/core-common";
import { MockRender } from "@itwin/core-frontend";
import { fireEvent, getByTestId, render } from "@testing-library/react";
import { BasemapPanel } from "../ui/widget/BasemapPanel";
import { defaultBaseMapLayers, SourceMapContext } from "../ui/widget/MapLayerManager";
import { TestUtils } from "./TestUtils";
Expand All @@ -19,7 +27,7 @@ describe("BasemapPanel", () => {
const sandbox = sinon.createSandbox();
const viewportMock = new ViewportMock();

const customBaseMap: coreCommon.BaseMapLayerSettings = coreCommon.BaseMapLayerSettings.fromJSON({
const customBaseMap: BaseMapLayerSettings = BaseMapLayerSettings.fromJSON({
formatId: "WMS",
name: "Custom Layer",
visible: true,
Expand All @@ -31,13 +39,13 @@ describe("BasemapPanel", () => {
});

before(async () => {
await coreFrontend.MockRender.App.startup();
await MockRender.App.startup();
await TestUtils.initialize();
window.HTMLElement.prototype.scrollIntoView = function () {};
});

after(async () => {
await coreFrontend.MockRender.App.shutdown();
await MockRender.App.shutdown();
TestUtils.terminateUiComponents();
});

Expand Down Expand Up @@ -65,11 +73,11 @@ describe("BasemapPanel", () => {
</SourceMapContext.Provider>,
);
await TestUtils.flushAsyncOperations();

const iconVisibility = container.querySelector(".icon-visibility");
const iconVisibility = getByTestId(container, "toggle-visibility");
// container.querySelector(".icon-visibility");
should().exist(iconVisibility);

const selectContent = container.querySelector(".iui-content") as HTMLElement;
const selectContent = getByTestId(container, "base-map-select") as HTMLElement;
should().exist(selectContent);
expect(selectContent.textContent).to.eql("WellKnownBaseMaps.BingProvider.Hybrid");
});
Expand All @@ -89,33 +97,33 @@ describe("BasemapPanel", () => {
</SourceMapContext.Provider>,
);

// let baseMap = coreCommon.BaseMapLayerSettings.fromProvider(coreCommon.BackgroundMapProvider.fromJSON({name: "BingProvider", type: coreCommon.BackgroundMapType.Street}));
let baseMap: coreCommon.BaseLayerSettings = defaultBaseMapLayers[2];
// let baseMap = BaseMapLayerSettings.fromProvider(BackgroundMapProvider.fromJSON({name: "BingProvider", type: BackgroundMapType.Street}));
let baseMap: BaseLayerSettings = defaultBaseMapLayers[2];
viewportMock.baseMap = baseMap;
viewportMock.onMapImageryChanged.raiseEvent(coreCommon.MapImagerySettings.fromJSON({ backgroundBase: baseMap }));
viewportMock.onMapImageryChanged.raiseEvent(MapImagerySettings.fromJSON({ backgroundBase: baseMap }));
await TestUtils.flushAsyncOperations();

let selectContent = container.querySelector(".iui-content");
let selectContent = getByTestId(container, "base-map-select");
should().exist(selectContent);
expect(selectContent!.textContent).to.eql("WellKnownBaseMaps.BingProvider.Street");

// Now test with a custom map-layer definition
baseMap = customBaseMap;
viewportMock.baseMap = baseMap;
viewportMock.onMapImageryChanged.raiseEvent(coreCommon.MapImagerySettings.fromJSON({ backgroundBase: baseMap }));
viewportMock.onMapImageryChanged.raiseEvent(MapImagerySettings.fromJSON({ backgroundBase: baseMap }));
await TestUtils.flushAsyncOperations();

selectContent = container.querySelector(".iui-content");
selectContent = getByTestId(container, "base-map-select");
should().exist(selectContent);
expect(selectContent!.textContent).to.eql(customBaseMap.name);

// Now test with a ColorDef
const color = coreCommon.ColorDef.create(coreCommon.ColorByName.aliceBlue);
const color = ColorDef.create(ColorByName.aliceBlue);
viewportMock.baseMap = color;
viewportMock.onMapImageryChanged.raiseEvent(coreCommon.MapImagerySettings.fromJSON({ backgroundBase: color.toJSON() }));
viewportMock.onMapImageryChanged.raiseEvent(MapImagerySettings.fromJSON({ backgroundBase: color.toJSON() }));
await TestUtils.flushAsyncOperations();

selectContent = container.querySelector(".iui-content");
selectContent = getByTestId(container, "base-map-select");
should().exist(selectContent);
expect(selectContent!.textContent).to.eql("Basemap.ColorFill");
});
Expand All @@ -135,12 +143,12 @@ describe("BasemapPanel", () => {
</SourceMapContext.Provider>,
);

const baseMap = coreCommon.BaseMapLayerSettings.fromProvider(
coreCommon.BackgroundMapProvider.fromJSON({ name: "BingProvider", type: coreCommon.BackgroundMapType.Street }),
const baseMap = BaseMapLayerSettings.fromProvider(
BackgroundMapProvider.fromJSON({ name: "BingProvider", type: BackgroundMapType.Street }),
{ invisible: true, transparency: 0.5 },
);
viewportMock.baseMap = baseMap; // mock needs to be updated too because the component refresh from the viewport too .
viewportMock.onMapImageryChanged.raiseEvent(coreCommon.MapImagerySettings.fromJSON({ backgroundBase: baseMap }));
viewportMock.onMapImageryChanged.raiseEvent(MapImagerySettings.fromJSON({ backgroundBase: baseMap }));
await TestUtils.flushAsyncOperations();

const iconVisibilityHide = container.querySelector(".icon-visibility-hide-2");
Expand All @@ -150,7 +158,7 @@ describe("BasemapPanel", () => {
const transparencyButton = container.querySelector(".map-transparency-popup-button") as HTMLElement;
should().exist(transparencyButton);
fireEvent.click(transparencyButton);
const sliderThumb = document.querySelector(".iui-slider-thumb");
const sliderThumb = document.querySelector('div[role="slider"]');
expect(sliderThumb?.getAttribute("aria-valuenow")).to.eql((baseMap.transparency * 100).toString());
});
});
51 changes: 25 additions & 26 deletions packages/itwin/map-layers/src/test/MapLayerManager.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@
import { expect, should } from "chai";
import * as sinon from "sinon";
import { ImageMapLayerSettings } from "@itwin/core-common";
import * as coreFrontend from "@itwin/core-frontend";
import { fireEvent, getAllByTestId, getByTestId, getByTitle, queryByText, render } from "@testing-library/react";
import { MapLayerIndex, MapLayerSource, MapLayerSources, MockRender } from "@itwin/core-frontend";
import { fireEvent, getAllByTestId, getByTestId, queryByText, render, RenderResult } from "@testing-library/react";
import { MapLayerPreferences, MapLayerSourceChangeType } from "../MapLayerPreferences";
import { MapLayerManager } from "../ui/widget/MapLayerManager";
import { TestUtils } from "./TestUtils";
Expand All @@ -29,13 +29,13 @@ describe("MapLayerManager", () => {
const sourceListSelector = ".map-manager-source-list";

before(async () => {
await coreFrontend.MockRender.App.startup();
await MockRender.App.startup();
await TestUtils.initialize();
window.HTMLElement.prototype.scrollIntoView = function () {}; // needed by <Select> UI component
});

after(async () => {
await coreFrontend.MockRender.App.shutdown();
await MockRender.App.shutdown();
TestUtils.terminateUiComponents();
});

Expand All @@ -51,7 +51,7 @@ describe("MapLayerManager", () => {
async function testSourceItems(testFunc: (menuItems: NodeListOf<HTMLLIElement>) => void, customDataset?: any, nbRender?: number, extraFunc?: () => void) {
sandbox.stub(MapLayerPreferences, "getSources").callsFake(async function (_iTwinId: GuidString, _iModelId?: GuidString) {
const dataset = customDataset ? customDataset : sourceDataset;
return dataset.map((source: any) => coreFrontend.MapLayerSource.fromJSON(source)!);
return dataset.map((source: any) => MapLayerSource.fromJSON(source)!);
});

render(
Expand Down Expand Up @@ -97,18 +97,19 @@ describe("MapLayerManager", () => {
<div>
<MapLayerManager getContainerForClone={() => document.body} activeViewport={viewportMock.object}></MapLayerManager>
</div>,
);
) as RenderResult;

await TestUtils.flushAsyncOperations();

viewportMock.onMapImageryChanged.raiseEvent({} as any);

const select = container.querySelector(".iui-input-with-icon") as HTMLElement;
const selectButton = select.querySelector(".iui-select-button") as HTMLElement;
const select = getByTestId<HTMLInputElement>(container, "base-map-select");
const selectButton = select.querySelector('div[role="combobox"]') as HTMLElement;
fireEvent.click(selectButton);
const menu = document.querySelector(".iui-menu") as HTMLUListElement;
const listboxes = container.querySelectorAll('div[role="listbox"]');
expect(listboxes.length).to.be.greaterThan(0);
const menu = listboxes[0] as HTMLUListElement;
should().exist(menu);
const menuItems = menu.querySelectorAll("li");
const menuItems = menu.querySelectorAll('button[role="option"]');

expect(menuItems.length).to.eq(4);
expect(menuItems[0].textContent).to.eql("Basemap.ColorFill");
Expand Down Expand Up @@ -138,9 +139,9 @@ describe("MapLayerManager", () => {
});

it("renders source list without duplicates", async () => {
const customDataset: coreFrontend.MapLayerSources[] = [...sourceDataset, sourceDataset[0]];
const customDataset: MapLayerSources[] = [...sourceDataset, sourceDataset[0]];
sandbox.stub(MapLayerPreferences, "getSources").callsFake(async function (_iTwinId: GuidString, _iModelId?: GuidString) {
return customDataset.map((source: any) => coreFrontend.MapLayerSource.fromJSON(source)!);
return customDataset.map((source: any) => MapLayerSource.fromJSON(source)!);
});

render(
Expand Down Expand Up @@ -177,7 +178,7 @@ describe("MapLayerManager", () => {
undefined,
1,
() => {
MapLayerPreferences.onLayerSourceChanged.raiseEvent(MapLayerSourceChangeType.Removed, coreFrontend.MapLayerSource.fromJSON(sourceDataset[0]));
MapLayerPreferences.onLayerSourceChanged.raiseEvent(MapLayerSourceChangeType.Removed, MapLayerSource.fromJSON(sourceDataset[0]));
},
);
});
Expand All @@ -195,8 +196,8 @@ describe("MapLayerManager", () => {
() => {
MapLayerPreferences.onLayerSourceChanged.raiseEvent(
MapLayerSourceChangeType.Replaced,
coreFrontend.MapLayerSource.fromJSON(sourceDataset[0]),
coreFrontend.MapLayerSource.fromJSON({ ...sourceDataset[0], name: renamedName }),
MapLayerSource.fromJSON(sourceDataset[0]),
MapLayerSource.fromJSON({ ...sourceDataset[0], name: renamedName }),
);
},
);
Expand All @@ -217,7 +218,7 @@ describe("MapLayerManager", () => {
undefined,
1,
() => {
MapLayerPreferences.onLayerSourceChanged.raiseEvent(MapLayerSourceChangeType.Added, coreFrontend.MapLayerSource.fromJSON(newSourceProps));
MapLayerPreferences.onLayerSourceChanged.raiseEvent(MapLayerSourceChangeType.Added, MapLayerSource.fromJSON(newSourceProps));
},
);
});
Expand Down Expand Up @@ -295,7 +296,7 @@ describe("MapLayerManager", () => {
const overlayLayerSetting = ImageMapLayerSettings.fromJSON({ ...backgroundLayerSettings.toJSON(), name: "overlay" });
viewportMock.backgroundLayers = [backgroundLayerSettings];
viewportMock.overlayLayers = [overlayLayerSetting];
viewportMock.detachMapLayerByIndexFunc = (mapLayerIndex: coreFrontend.MapLayerIndex) => {
viewportMock.detachMapLayerByIndexFunc = (mapLayerIndex: MapLayerIndex) => {
mapLayerIndex.isOverlay ? (viewportMock.overlayLayers = []) : (viewportMock.backgroundLayers = []);
};
viewportMock.setup();
Expand All @@ -310,7 +311,7 @@ describe("MapLayerManager", () => {
const checkLayerSection = async (section: HTMLElement, sectionName: string) => {
let listItem = queryByText(container, sectionName);
should().exist(listItem);
const detachAllButton = getByTitle(section, "MapLayerActionButtons.DetachSelectedLabel");
const detachAllButton = getByTestId(section, "detach-label-button");
should().exist(detachAllButton);

const checkbox = getByTestId(section, "select-item-checkbox");
Expand Down Expand Up @@ -364,23 +365,21 @@ describe("MapLayerManager", () => {
checkLayerItemsVisibility(section, 1, 0);

// Click on the HideAll it should change the eye icon

// const hideAllButtons = getAllByTitle(container, "MapLayerActionButtons.HideAllLabel");
const hideAllButton = getByTitle(section, "MapLayerActionButtons.HideAllLabel");
const hideAllButton = getByTestId(section, "hide-all-label-button");
should().exist(hideAllButton);
hideAllButton.click();
await TestUtils.flushAsyncOperations();
checkLayerItemsVisibility(section, 0, 1);

// Click on the HideAll it should change the eye icon
const showAllButton = getByTitle(section, "MapLayerActionButtons.ShowAllLabel");
// Click on the ShowAll it should change the eye icon
const showAllButton = getByTestId(section, "show-all-label-button");
should().exist(showAllButton);
showAllButton.click();
await TestUtils.flushAsyncOperations();
checkLayerItemsVisibility(section, 1, 0);

// Click on the HideAll it should change the eye icon
const InvertButton = getByTitle(section, "MapLayerActionButtons.InvertAllLabel");
// Click on the InvertAll it should change the eye icon
const InvertButton = getByTestId(section, "invert-all-label-button");
should().exist(InvertButton);
InvertButton.click();
await TestUtils.flushAsyncOperations();
Expand Down
Loading
Loading