Move ThumbnailFactory and manifesto.js wrappers into selectors and hooks#4102
Conversation
barmintor
commented
Feb 12, 2025
barmintor
commented
- prepare for auth2 service parsing changes
- prevent exposing wrapper constructor changes to wrappers to broad swathes of app
- stabilize empty object/array references for input selectors
- fixes Changes to manifesto.js wrapper constructors affect code in much of application #4100
- prepare for auth2 service parsing changes - prevent exposing wrapper constructor changes to wrappers to broad swathes of app - stabilize empty object/array references for input selectors - fixes ProjectMirador#4100
|
@cbeer this is one way to address the issue of config references propagating everywhere if they're added to the MiradorCanvas constructor. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4102 +/- ##
==========================================
+ Coverage 95.07% 95.09% +0.01%
==========================================
Files 317 322 +5
Lines 15973 16038 +65
Branches 2509 2521 +12
==========================================
+ Hits 15187 15252 +65
Misses 782 782
Partials 4 4 ☔ View full report in Codecov by Sentry. |
| describe('getThumbnailFactory', () => { | ||
| const state = { config: miradorConfigSlice() }; | ||
| const iiifOpts = {}; | ||
| it('returns the manifest of a certain id', () => { |
There was a problem hiding this comment.
Is "the manifest of a certain id" being returned in this test? It seems to expect ThumbnailFactory.
There was a problem hiding this comment.
You're right. Do you want to suggest a change here, or do you want me to push an additional commit?
There was a problem hiding this comment.
For some reason I can't suggest a change. Can you push a commit?
src/state/selectors/layers.js
Outdated
| import { getCanvas, getVisibleCanvasIds } from './canvases'; | ||
| import { miradorSlice } from './utils'; | ||
| import { miradorSlice, EMPTY_ARRAY } from './utils'; | ||
| import { getConfig } from './config'; |
There was a problem hiding this comment.
getConfig import is not used in this file
There was a problem hiding this comment.
that's weird - why didn't the linter catch this?
|
Just a question @barmintor, does the "stabilize empty object/array references for input selectors" work you've done address the |
|
@marlo-longley it addresses some, but not all, of those warnings |
3 participants
