search

mapbox / mapbox-gl-native

Interactive, thoroughly customizable maps in native Android, iOS, macOS, Node.js, and Qt applications, powered by vector tiles and OpenGL
https://mapbox.com/mobile
Other
4.37k stars 1.33k forks source link

Missing outlet warning at launch in macosapp #14389

Closed 1ec5 closed 5 years ago

1ec5 commented 5 years ago

The following warning appears in the console when launching macosapp as of 8389e746b6745a68fcd58ece8e398bde0a85b57f:

2019-04-10 13:41:56.090895-0700 Mapbox GL[30223:7451050] Failed to connect (includeIdeographsBox) outlet from (MapDocument) to (NSButton): missing setter or instance variable

/cc @fabian-guerra

1ec5 commented 5 years ago

The “Include CJK characters” checkbox in the Add Offline Pack sheet is hooked up to MapDocument.includeIdeographsBox, which isn’t defined.

13607 added this checkbox to the sheet but defined the outlet as includesIdeographicGlyphsBox:

https://github.com/mapbox/mapbox-gl-native/blob/d3bb518976a8d99c36c627e5be907065497dfa58/platform/macos/app/MapDocument.m#L85

It was probably a last-minute refactoring, but it means that the checkbox has been nonfunctional all this time. Any offline pack size estimates that depended on this checkbox being unchecked are inaccurate.

/cc @chloekraw @coxchapman

chloekraw commented 5 years ago

Thanks for catching this and fixing it so quickly @1ec5! Is this the kind of thing we can or should add tests for?

cc/ @mapbox/maps-ios

1ec5 commented 5 years ago

This only surfaced as a runtime warning, since it was an application-level UI bug. We’d need to create an UI test integration target like we have for iOS. It’d be a bit of a big lift, versus educating contributors to this codebase that XIBs can have code-level dependencies on source files.