search

Esri / military-symbology

A user-focused add-in for searching, creating, and editing military symbols in ArcGIS Pro.
Apache License 2.0
39 stars 10 forks source link

Support APP6B in MSE #373

Closed lfunkhouser closed 5 years ago

lfunkhouser commented 5 years ago

Support APP6B

csmoore commented 5 years ago

Note: added a dependency on issue #362 / PR #366 - I can continue development with a local copy of the lpkx from this PR, but this PR would just need merged before this issue be tested.

csmoore commented 5 years ago

Adding this to the MSE seems to be working as expected ( in branch )

The only minor issue I am seeing (so far) is the issue we saw with some of the symbol names not being fully descriptive - https://github.com/Esri/military-symbology/issues/362#issuecomment-488819577 - and changed to the lpkx in https://github.com/Esri/military-symbology/commit/8d53937d157acc2f31e3d7ea2c836f092174a07c

Example:

image

In the MSE, if you scroll down to the tags, you can get some more info on the symbol to be able to disambiguate so this issue may be more minor, and not sure if the stylx symbol names need to be updated

image

csmoore commented 5 years ago

PR #375 submitted to add this support. Important Testing Note: Favorites will not work until #374 is addressed.

dfoll commented 5 years ago
csmoore commented 5 years ago

The missing echelons turns out to be a problem with our a couple domain values having special characters (tabs/spaces) - fixing these domain values will fix this:

image

The missing mobility modifiers may be a problem we never caught with the updated arcade renderer, I am seeing this problem with 2525B/C also.

lfunkhouser commented 5 years ago

Update Echelon label to "Echelon/Mobility"

csmoore commented 5 years ago

Update PR #375 with most of the testing feedback.

New issues were created for the 2 issues that were not addressed:

  1. Missing mobility modifiers -seems to be renderer issue: https://github.com/ArcGIS/military-symbology-styles/issues/193
  2. Better restrict domain values for echelon/mobility - would be a larger fix and applies to other standards also: #377

Sorry for the delay with this - I had a problem with packaging that only seems to show up on my machine (contacted Core)

dfoll commented 5 years ago

@csmoore I need some help on this one. When I get build 111 which is the most recent from this branch I am not seeing APP6B in the drop-down when trying to add a schema to the map

csmoore commented 5 years ago

@dfoll - it does require 2.4 to see the app6b option - unless it is a bug

dfoll commented 5 years ago

@csmoore my bad, was so sure that i had 2.4 on my system that I didn't even consider that as the problem.

dfoll commented 5 years ago

@csmoore between earlier testing, and confirming the few small issues i found are fixed, is there anything in particular you would like me to dig deeper on? If not I am ready to move on to documentation for this issue

dfoll commented 5 years ago

TODO (doc work):

Github based edits in this PR

dfoll commented 5 years ago

TODO (doc verify):

dfoll commented 5 years ago

I have done the majority of this documentation. left the doc tags on awaiting response to email from @lfunkhouser @csmoore @kgonzago @ljuru after i get that response, we can fully remove all doc labels except for qc

lfunkhouser commented 5 years ago

Noticed there are no tests for the MSE that test APP-6. Added label to create tests.

lfunkhouser commented 5 years ago

doc reviewed. MSE verified on ...\MilitarySymbolEditor\Dev\119 MT verified on ...\MilitaryToolsForArcGIS\Dev\308 website doc updates verified on solutionsdev Marking this issue as done. Issue to update tests is in the test repo, ...solutions-defense-test-catalog/issues/212