fix(PdfReader): make FitMode work #898
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Reviewer's guide (collapsed on small PRs)Reviewer's GuideAdjusts how PdfReader’s FitMode is passed from the .NET component to the JavaScript viewer and fixes its usage during pages initialization so the configured fit mode is properly applied. Sequence diagram for PdfReader FitMode initialization flowsequenceDiagram
participant BlazorApp
participant PdfReaderComponent
participant JSRuntime
participant PdfReaderJs
participant PdfViewer
BlazorApp->>PdfReaderComponent: Render PdfReader
PdfReaderComponent->>PdfReaderComponent: InvokeInitAsync()
PdfReaderComponent->>PdfReaderComponent: FitMode.ToDescriptionString()
PdfReaderComponent->>JSRuntime: InvokeAsync Init(options with FitMode string)
JSRuntime->>PdfReaderJs: initPdfReader(options)
PdfReaderJs->>PdfViewer: create pdfViewer with options
PdfReaderJs->>PdfReaderJs: addEventBus(el, pdfViewer, eventBus, invoke, options)
PdfReaderJs->>PdfReaderJs: eventBus.on(pagesinit)
PdfViewer-->>PdfReaderJs: pagesinit
PdfReaderJs->>PdfReaderJs: if options.fitMode
PdfReaderJs->>PdfViewer: currentScaleValue = options.fitMode
Class diagram for PdfReader FitMode option handlingclassDiagram
class PdfReaderComponent {
string Url
object _data
string FitMode
bool EnableThumbnails
int CurrentPage
bool TriggerPagesInit
Task InvokeInitAsync()
}
class PdfReaderOptions {
string Url
object Data
string FitMode
bool EnableThumbnails
int CurrentPage
bool TriggerPagesInit
}
class PdfReaderJsModule {
void initPdfReader(string elementId, PdfReaderOptions options)
}
class PdfViewer {
string currentScaleValue
int pagesCount
}
PdfReaderComponent ..> PdfReaderOptions : builds
PdfReaderComponent ..> PdfReaderJsModule : uses
PdfReaderJsModule ..> PdfViewer : configures
PdfReaderOptions --> PdfViewer : options.fitMode applied to currentScaleValue
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- Passing
FitMode.ToDescriptionString()through to JS couples behavior to the enum’s description attributes; consider using a stable enum name or explicit mapping so refactors of display text don’t silently break the viewer integration. - The
if (options.fitMode)guard inpagesinitrelies on a truthy check; if a valid fit mode could be an empty string or other falsy value, consider an explicit null/undefined check instead to avoid skipping the scale assignment.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Passing `FitMode.ToDescriptionString()` through to JS couples behavior to the enum’s description attributes; consider using a stable enum name or explicit mapping so refactors of display text don’t silently break the viewer integration.
- The `if (options.fitMode)` guard in `pagesinit` relies on a truthy check; if a valid fit mode could be an empty string or other falsy value, consider an explicit null/undefined check instead to avoid skipping the scale assignment.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug where the FitMode parameter was not working correctly in the PdfReader component. The issue was caused by two problems: the JavaScript code referenced an undefined variable instead of the options object, and the C# code was passing an enum value instead of its string representation.
- Fixed JavaScript to use
options.fitModeinstead of undefinedfitModevariable - Fixed C# to convert the enum to its string description using
ToDescriptionString()
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| PdfReader.razor.js | Corrected variable reference from fitMode to options.fitMode in the event handler |
| PdfReader.razor.cs | Added .ToDescriptionString() conversion to pass the enum as a string to JavaScript |
| BootstrapBlazor.PdfReader.csproj | Bumped version from 10.0.24 to 10.0.25 for bug fix release |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Link issues
fixes #897
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Ensure PdfReader applies the configured FitMode when initializing the PDF viewer.
Bug Fixes: