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.

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

Enhance applySavedView API #99

Merged
merged 2 commits into from
Nov 14, 2024
Merged

Enhance applySavedView API #99

merged 2 commits into from
Nov 14, 2024

Conversation

roluk
Copy link
Member

@roluk roluk commented Nov 12, 2024

Rework applySavedView settings interface to make it easier to understand.

Initially I designed it around the idea of extensions having three three operations: save, apply, and reset. While this system worked well for some applications, this confined the interface to a narrow applicability space and burdened users with a mental model that didn't necessarily match their expectations. This change is a step towards rethinking what it means to "apply a saved view", and making built-in EmphasizeElements and PerModelCategoryVisibility extensions first-class concepts of saved-views-react.

Breaking changes

  • Make applySavedView settings easier to understand
    • Remove "reset" from ApplyStrategy union, instead make "clear" a valid value for emphasis and perModelCategoryVisibility properties
    • Remove all property which set default ApplyStrategy of all settings
    • Update documentation

Minor changes

  • applySavedView enhancements
    • Accept custom viewChangeOptions that the function will internally pass through to viewport.changeView call
    • Add camera setting that controls how camera data is applied. Allow supplying a custom ViewPose or ignoring Saved View data to keep the camera in place.

Other

  • Improve documentation
    • Add precision, change wording, and simplify examples
    • Remove mentions of savedviews:read and savedviews:modify because they have been superseded by itwin-platform scope
  • Reformat code where it exceeds 100 columns in width, down from 120
    • Start breaking comments into new lines after reaching 80th column
  • Simplify executeQuery use by making it return a flat list of ids

Copy link
Collaborator

@diegopinate diegopinate left a comment

Choose a reason for hiding this comment

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

Looks good! I'd like to discuss your thoughts on how we should handle generic extensions later on.

@roluk roluk merged commit efc4b82 into master Nov 14, 2024
4 checks passed
@roluk roluk deleted the enhance-view-api branch November 14, 2024 12:46
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.

2 participants