Skip to content

Naron/overall loading#222

Merged
naronchen merged 12 commits into
masterfrom
naron/overall-loading
May 20, 2025
Merged

Naron/overall loading#222
naronchen merged 12 commits into
masterfrom
naron/overall-loading

Conversation

@naronchen

Copy link
Copy Markdown
Contributor

@naronchen naronchen requested a review from a team as a code owner May 7, 2025 21:48
@naronchen naronchen requested a review from diegopinate May 7, 2025 21:48
@changeset-bot

changeset-bot Bot commented May 7, 2025

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 79f15aa

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@itwin/changed-elements-react Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Comment thread packages/changed-elements-react/src/api/ChangedElementEntryCache.ts
Comment thread packages/changed-elements-react/src/api/ChangedElementEntryCache.ts Outdated
Comment thread packages/changed-elements-react/src/api/ChangedElementEntryCache.ts Outdated
Comment thread packages/changed-elements-react/src/api/VersionCompareManager.ts
Comment thread packages/changed-elements-react/src/api/VersionCompareManager.ts Outdated
Comment thread packages/changed-elements-react/src/api/VersionCompareManager.ts Outdated
Comment thread packages/changed-elements-react/src/api/VersionCompareManager.ts Outdated
Comment thread packages/changed-elements-react/src/widgets/ChangedElementsWidget.tsx Outdated
@naronchen

naronchen commented May 8, 2025

Copy link
Copy Markdown
Contributor Author

Overview:

In NameVersionSelectWidget there's a isComparing flag that returns NamedVersionSelector at first. When user click to view a comparison, the startComparisonV2 gets triggered, running async, it triggers this.versionCompareStarting.raiseEvent() at the beginning, which flipped the flag.

the flag flip will render ChangedElementWidget, there the listeners for loading messages gets hooked in. but we have some events already getting triggered in startComparisonV2 which has gone to void.

The result of this:

from my testing, listener gets hooked in on CheckpointConnection.openRemote, so if downloading the imodel takes a long time, the loading message will stay as 0%.

Diego mentioned its fine that listener hooked in later. But i thought its still worth documenting this and putting the problem out :)

Comment thread packages/changed-elements-react/src/widgets/ChangedElementsWidget.tsx Outdated
Comment thread packages/changed-elements-react/src/widgets/ChangedElementsWidget.tsx Outdated
Comment thread packages/changed-elements-react/src/widgets/ProgressCoordinator.ts
Comment thread packages/changed-elements-react/src/api/ElementQueries.ts Outdated
Comment thread packages/changed-elements-react/src/api/ChangedElementEntryCache.ts Outdated
Comment thread packages/changed-elements-react/src/api/ChangedElementEntryCache.ts Outdated
@naronchen naronchen requested review from a team and CalebGerman May 9, 2025 17:44
CalebGerman
CalebGerman previously approved these changes May 12, 2025
Comment thread packages/changed-elements-react/src/api/ChangedElementEntryCache.ts Outdated
Comment thread packages/changed-elements-react/src/api/ChangedElementEntryCache.ts Outdated
Comment thread packages/changed-elements-react/src/widgets/ProgressCoordinator.ts Outdated
diegopinate
diegopinate previously approved these changes May 20, 2025
@naronchen naronchen merged commit 0c64370 into master May 20, 2025
4 checks passed
@naronchen naronchen deleted the naron/overall-loading branch May 20, 2025 14:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants