Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuidePdfReader viewer control wiring is updated to reference the correct scale input element CSS class and the component package version is bumped to 10.0.2. Sequence diagram for PdfReader scalechanging handlingsequenceDiagram
actor User
participant PdfReaderElement as PdfReaderElement
participant ZoomControls as ZoomControls
participant PdfViewer as PdfViewer
participant EventBus as EventBus
participant JsHandler as PdfReaderJs
User->>PdfReaderElement: Click bb-page-plus or bb-page-minus
PdfReaderElement->>ZoomControls: Trigger zoom control handler
ZoomControls->>PdfViewer: Request zoom change
PdfViewer-->>EventBus: Emit scalechanging(scale)
EventBus-->>JsHandler: scalechanging event
JsHandler->>PdfReaderElement: querySelector(.bb-view-scale-input)
JsHandler->>ZoomControls: Update scale input value
JsHandler->>ZoomControls: Update other zoom UI state
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- Consider keeping backward compatibility by querying both the new
.bb-view-scale-inputand the previous.bb-view-scaleclass (fallback if the new one is not found) to avoid breaking existing markup. - It may be safer to guard against
scaleElbeingnullbefore using it in the event handlers, so a missing or misnamed element doesn’t cause runtime errors.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Consider keeping backward compatibility by querying both the new `.bb-view-scale-input` and the previous `.bb-view-scale` class (fallback if the new one is not found) to avoid breaking existing markup.
- It may be safer to guard against `scaleEl` being `null` before using it in the event handlers, so a missing or misnamed element doesn’t cause runtime errors.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 in the PdfReader component where the JavaScript code was incorrectly querying for a container div (.bb-view-scale) instead of the actual input element (.bb-view-scale-input) that displays the zoom scale percentage. The patch version is bumped from 10.0.1 to 10.0.2 to reflect this bug fix.
Key changes:
- Corrected JavaScript selector to target the scale input element instead of its parent container
- Bumped version to 10.0.2 following semantic versioning for a bug fix
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| PdfReader.razor.js | Fixed selector to query .bb-view-scale-input (the input element) instead of .bb-view-scale (the container div), ensuring the scale value is correctly set on the input element |
| BootstrapBlazor.PdfReader.csproj | Bumped version from 10.0.1 to 10.0.2 for this bug fix release |
Comments suppressed due to low confidence (1)
src/components/BootstrapBlazor.PdfReader/PdfReader.razor.js:247
- The initial value of v is unused, since it is always overwritten.
let v = 100;
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Link issues
fixes #734
Summary By Copilot
Regression?
Risk
Verification
Packaging changes reviewed?
☑️ Self Check before Merge
Summary by Sourcery
Align PdfReader scale control wiring with updated DOM structure and version metadata.
Bug Fixes:
Build: