Skip to content

Move the bannerStateForResults helper into the SignaturePropertiesManager class (PR 21247 follow-up)#21533

Merged
timvandermeij merged 1 commit into
mozilla:masterfrom
Snuffleupagus:SignaturePropertiesManager-#bannerState
Jul 3, 2026
Merged

Move the bannerStateForResults helper into the SignaturePropertiesManager class (PR 21247 follow-up)#21533
timvandermeij merged 1 commit into
mozilla:masterfrom
Snuffleupagus:SignaturePropertiesManager-#bannerState

Conversation

@Snuffleupagus

@Snuffleupagus Snuffleupagus commented Jul 3, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

This has a couple of advantages:

  • It allows accessing and iterating this.#results directly, without having to (needlessly) create a temporary array.

  • By moving the "worst status" computation into its own getter, it can be re-used from the #updateButtonState method as well which reduces code duplication.

@codecov-commenter

codecov-commenter commented Jul 3, 2026 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.42%. Comparing base (0cc1718) to head (14f4cce).

Files with missing lines Patch % Lines
web/digital_signature_properties_manager.js 0.00% 15 Missing ⚠️
Additional details and impacted files 
@@           Coverage Diff           @@
##           master   #21533   +/-   ##
=======================================
  Coverage   89.41%   89.42%           
=======================================
  Files         262      262           
  Lines       66760    66756    -4     
=======================================
+ Hits        59691    59694    +3     
+ Misses       7069     7062    -7
Flag Coverage Δ
browsertest 66.52% <ø> (-0.02%) ⬇️
integrationtest 68.73% <0.00%> (-0.01%) ⬇️
unittest 57.33% <ø> (+<0.01%) ⬆️
unittestcli 56.30% <ø> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Snuffleupagus Snuffleupagus marked this pull request as draft July 3, 2026 10:33
@Snuffleupagus
Move the bannerStateForResults helper into the `SignatureProperties…
14f4cce 
…Manager` class (PR 21247 follow-up)

This has a couple of advantages:
 - It allows accessing and iterating `this.#results` directly, without having to (needlessly) create a temporary array.

 - By moving the "worst status" computation into its own getter, it can be re-used from the `#updateButtonState` method as well which reduces code duplication.
@Snuffleupagus Snuffleupagus force-pushed the SignaturePropertiesManager-#bannerState branch from 1f9d320 to 14f4cce Compare July 3, 2026 10:45
@Snuffleupagus Snuffleupagus marked this pull request as ready for review July 3, 2026 10:50
@timvandermeij timvandermeij merged commit 32f3906 into mozilla:master Jul 3, 2026
17 checks passed
@timvandermeij

timvandermeij commented Jul 3, 2026

Copy link
Copy Markdown
Collaborator

Good idea; thanks!

@Snuffleupagus Snuffleupagus deleted the SignaturePropertiesManager-#bannerState branch July 4, 2026 10:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@timvandermeij timvandermeij timvandermeij approved these changes

Assignees

No one assigned

Labels

viewer

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Snuffleupagus @codecov-commenter @timvandermeij