Core refactor #961

Trying to get to a place where all objects are just owned by context.

PixiJS uses this "system" naming convention so we will too

Trying to get to a place where all objects are just owned by context. This includes quite a lot of changes to the preset/field system: - All the preset related components are ES6 classes now - Stuff moved from modules/presets -> modules/core - Generally, tried to simplify the code as much as possible and avoid "magic" - Lots of changes around Collection, now it just contains an `array` property, no more pretending Collections is an Array or PresetIndex is a Collection - Some things that were functions before (setAddablePresetIDs) are just properties now - uiField -> UiField is now an ES6 class too - Most of the fields have been rewritten for ES6, many revisions and updates - The old way of converting a field -> presetfield -> uifield was pretty complicated, now I try to explicitly copy over properties from one component to another and call out places where a class property gets replaced by a class function Also fixed an unrelated bug in uiIcon / disclosure that was causing the hide/show toggle icons to not appear (because I removed third argument to UiIcon in 05f2f34)

All will move to ES6 classes that have reference back to context and follow predictable setup and initialization flow. No more magic contextless globals.

Also removing maprules - I dont think anyone uses it anymore.

Fixed the missing links to the preset and location systems now that NSI service has a reference back to context

Still need to fix the show/hide viewer code

This is all of them! Still need to restore some tests

Mostly this is just using fat arrow functions to preserve `this`

Rapid does work, but now the tests are a mess. This is because the interactions between the localizer and everything else get complicated, as creating a context will start loading data and the fileFetcher isn't global anymore. The tests did have a lot of code in spec_helpers to prevent files from being fetched, that doesn't work now. This can start getting easier after we change the localizer similarly.

This is a big change: - Eliminated the global `localizer` and `t()` functions - Added `context.t()` and `context.tHtml()` convenience functions - Some util functions relied on the global `t()` for localization (e.g. utilDisplayName, etc) these are moved into the LocalizationSystem now - Some more things need context now to avoid using the global `t()` - `ImagerySource` now passed a context - `ActionMergeRemoteChanges` rewritten to take an options param with the localizer - several more UI classes take context now - Reordered some arglists (e.g. for uiPane), context should always be the first one - Lots of changes to the tests. They were loading the general en locale strings before and they should not do this. - Still some more work to do on tests to be better at mocking context

- Restored the service tests that I didn't do before (Osm, Kartaview, Mapillary, Streetside) - Fixed the other service tests that were failing because of missing fetch mock resets - Removed the sticky fetch mock routes (they were only needed by serviceosm capabilities test) - Also went through and simplified a bunch of the validator tests - We don't need to create a full context for these - We don't need to run them async to wait for data to load

This commit also moves a bunch of stuff from /core -> /core/lib for organization

I also got the old RendererMap tests working again. These were skipped, but they can work now with some substantial mocking. Mostly because MapSystem listens to events emitted by many other things. It might make more sense to avoid these listeners and instead have the other systems tell them map to redraw as needed. We have map.deferredRedraw() now so this can be done in a less intrusive way. Also, we still have to sort out initialization of the map a bit better, but we're getting there.

Following up on an idea from 1d5e617, Just call `mapSystem.deferredRedraw()` or `mapSystem.immediateRedraw()` instead of listening to events from various places. This much simplifies the various Pixi layers that were registering listeners on the service objects. It also makes mocking mapSystem a little easier.

This is a big change but will make the code easier to work with. Some linger issues: - Switching from d3_dispatch to EventEmitter means we have to change some of the listeners in the walkthrough - Some of the listeners seem to be firing too much - `reset` used to be used for both resetting but also returning to a saved checkpoint. I split these into two functions. - It used to dispatch a 'reset' event that was only used by the validator. The validator also has 2 resets, one that resets everything and one that preserves the user's ignored issues. This is also something we can clean up.

Mostly in the walkthrough

- It's pointless to requestIdleCallback the osm api calls that will be async anyway - We're at the point where we can get rid of the distinction between instantiate and initialize in the `.init()` function All core classes can be instantiated earlier now and in any order, as they have class constructors that don't depend on each other for anything.

By using smaller chunks, we will block the main cpu for shorter amounts of time, allowing the browser to be more responsive. Eventually I'd like to move this to a worker, but we don't have a great way of sharing the context/graph/edits with worker processes yet.

e.g. "MoveMode" not "ModeMove" The former is more how we would speak about and think about the thing. We already have the core systems named this way (e.g. "EditSystem") so we may as well do it for all the things.

Behaviors -> AbstractBehavior -> EventEmitter Modes -> AbstractMode -> EventEmitter Services -> AbstractService -> EventEmitter Systems -> AbstractSystem -> EventEmitter

This would allow a custom install of Rapid to add/replace/remove these components before context.init() is called. Maybe an early first step towards supporting plugins and other customization.

Also replace the d3-dispatch('enter','exit') with EventEmitter and just emit 'modechange'

I thought about making these static. For modes it's easier for them to be class properties than static properties. (i.e. we need to get `mode.id` in several places)

I'd still rather not have 2 select modes, but at least the modes are more standardized now.

- context, ui, validations all had a lot of work to do (many tests that were skipped before will now run!) - We avoid creating a new Context() now just to get unit tests to work Instead, mock out the systems that the unit tests need.

localStorage can only store strings, so before if we passed it a null, it stored 'null'. Then on a restart we'd see the 'null' there and think there were changes to restrore.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Core refactor #961

Core refactor #961

Commits on Jun 20, 2023