Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Responsive panel-based UI #99

Merged
merged 82 commits into from
Sep 21, 2020
Merged

Responsive panel-based UI #99

merged 82 commits into from
Sep 21, 2020

Conversation

nCastle1
Copy link
Collaborator

@nCastle1 nCastle1 commented Jul 31, 2020

Updates Data Collection with a new panel-based UI and a modified design for both UWP and WPF.

Note: need to figure out why template is being ignored on UWP
…ion-dotnet into ncastle/panel-based-ui

# Conflicts:
#	src/ControlsTestWPF/MainWindow.xaml
Improved appearance of card icons, attribution; added attribution popup viewer; Fixed window maximize behavior by adding controlzEx and using window behaviors from Fluent.Ribbon project
@esreli
Copy link
Contributor

esreli commented Aug 5, 2020

Nathan you've outdone yourself with this one, the work you've done here is superb. You've added polish to the existing app, supported by an architecture and have re-designed UIs in ways that suit a desktop design far better than ever before. Bravo, your contribution here cannot be understated.

I've divided up this review into two categories. The first, New UI Architecture & Design, are comments I have surrounding your new UI architecture and designs. The second, Found Bugs, are general bugs i've found while reviewing the app.

New UI Architecture & Design

Accent Color

It appears we have two greens competing for accent color. For instance

  • #35AC46 was found in various UI button elements, the title bar and add feature button
  • #60993D was found accenting view boxes and borders, in the pop-up view

Is this intentional? Are they designed to convey different UI element types?

Home Button

Screen Shot 2020-08-04 at 3 51 50 PM

It's not intuitive to me what this button does. Knowing the dataset and Runtime more generally, it's my assumption that this set's the map view viewpoint to the map's initial viewpoint. Is this correct?

I wonder if there's a more effective way to communicate this feature?

It's also entirely possible that a home button is intuitive to most people and that i'm overthinking here.

Title Bar Button

Screen Shot 2020-08-04 at 3 56 44 PM

I really like how you've created this UI element to convey the app's online/offline work mode status. I love the typography/iconography combo as well as the use of color and font weight to convey state and map title. Very nice.

One question here, is it commonly understood among windows users that they can tap the title like a button and expect it to respond? I'm not sure I would intuitively know this is a button without being told or stumbling across the feature by hovering over the title. On the other hand, once it becomes clear the feature conveys quite well!

ArcGIS Online Calcite Icon

The chosen calcite icon is a good one but might be just a tad misleading. To me, this icon conveys "has connection" rather than "working online".

Screen Shot 2020-08-04 at 5 27 56 PM

We might want to consider replacing the icon with one that represents 'ArcGIS Online' or 'Portal'.

Screen Shot 2020-08-04 at 5 30 36 PM

Please feel free to throw this suggestion away. What you've come up with works well, i'm just curious if it could work even better.

Compass

I can't figure out how to rotate the map and reveal the compass. How do I do so using a keyboard/mouse?

Map Info

Screen Shot 2020-08-04 at 4 17 20 PM

Created Date Formatting

Can we modify the formatted date for one that reads a bit more smoothly? Perhaps something like:

MM/DD/YYYY, hh:mm

If this can be localized, even better.

Map Description

Where does this description come from?

Working Offline

Screen Shot 2020-08-04 at 4 31 38 PM

The title label switches to gray, I think this is a great touch toward communicating the app work mode status.

It appears the map's title disappears, the title bar now only reads 'Working Offline'. Can we put the map title back in here? Also, can you apply the same font weight as you do when the app is working in online mode?

Add Related Record

Can you rename the 'Add Related Record' button to format with the name of the related table's table name? From the above example that would read 'Add Inspections'

Screen Shot 2020-08-04 at 4 42 43 PM

M:1 Related Records Section Divider

I think I understand why the section divider that reads 'Linked Records.' It's my preference that this divider is removed though I'm open to it staying if you want to suggest a compelling reason. Or, perhaps, we replace 'Linked Records' with a title of the related feature table's table name? What are your thoughts here?

Screen Shot 2020-08-04 at 4 44 27 PM

Attribute Box

I'd like to devise a technique for communicating if an attribute field is currently in edit mode or not. To me, the border color implies a field is click-able and if the field is click-able, it is likely editable.

Perhaps the border color defaults to none (or the same color as the attribute background) and adds a border color like you have in the design when the pop-up is in an active editing session?

Screen Shot 2020-08-04 at 4 50 17 PM

Related Records Border Boxes

Can we remove the green border surrounding an in-line related record? I think i'd prefer it to match the design of an attribute's box.

Screen Shot 2020-08-04 at 4 48 32 PM

Pop-up Date Field

The pop-up date field background color when editing does not match that of it's containing box. Can we get these two to match?

Screen Shot 2020-08-04 at 4 55 16 PM

Tab-Context Add Attachment Folder

Can we have the tab drive what toolbar buttons live in the pop-up view's bottom toolbar?

Specifically, I think it's appropriate for the 'Attachments' tab to show the add attachment folder button. The same button persists when viewing the 'Details' tab and I think it's a bit misleading. Would you remove that toolbar when viewing the 'Details' tab and include it when viewing the 'Attachments' tab?

Details Attachments
(removed) (included)
Screen Shot 2020-08-04 at 5 20 09 PM Screen Shot 2020-08-04 at 5 20 16 PM

Found Bugs

Layers

Layers TOC tool is missing its title. What title do you suggest we use? My vote would be for "Layers" though I can be swayed.

Screen Shot 2020-08-04 at 4 01 30 PM

Bookmarks

Bookmarks tool is missing its title.

Screen Shot 2020-08-04 at 4 06 14 PM

Empty Sign Out view

I clicked 'Sign-out' and this box persisted on the screen.

Screen Shot 2020-08-04 at 4 11 18 PM

Download Map Offline

Screen Shot 2020-08-04 at 4 25 12 PM

The map info/ download map offline button persists after clicking 'download map offline'.

Pop-up 'Site Width' Validation

When the text field is blank, there appears to be attribute editing validation thrown. This behavior does not exist in the iOS app counterpart, of course this is not to say the iOS app isn't experiencing a bug.

It was my impression that this field was nullable and as such shouldn't throw a validation error. Can you look into what's happening here?

Screen Shot 2020-08-04 at 4 57 38 PM

Pop-up: Details: Attachments

While editing the pop-up, I clicked the folder icon which prompted me to choose a file from the windows file manager. Then, I clicked 'Cancel' and the content rendered in the 'Details' tab has disappeared. What's more, closing the pop-up view and performing a new identify does not restore the 'Details' tab's view.

Screen Shot 2020-08-04 at 5 05 26 PM

Copy link
Contributor

@esreli esreli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nCastle1 I think we're nearly there with this latest submission. I'm approving this PR with the assumption that the sole bug is addressed (or acknowledged). Congrats, this project is huge!

I've divided this review into two sections. The first, Bugs, is a slight misrepresentation because i've found only one bug that hasn't already been raised- and i'm not entirely sure it's a bug.

The second, Platform Parity, is a section devoted to requests i'm making that bring the application in its current state to parity with other Data Collection apps. It's very important to emphasize here that these suggestions are not coupled to this PR. However, they are a requirement for the release of v1.3. As such if you feel as though they can be addressed at a later time, please feel free to extract them from this review into their own issues. Also, i'd be happy to expand on these requirements if anything is unclear or you're needed a more rigid spec req, please just ask.


Bugs

Species Related Record Odd Behavior

I'm experiencing odd behavior when editing the Species related record. When editing the Tree record, after selecting a new Species from the drop down and before saving the Tree record, the Species 'field' becomes wildly scrollable. This probably doesn't make sense, here are steps to repro:

  1. Identify Tree
  2. Start editing session
  3. Click species field dropdown
  4. Select new species
  5. Hover mouse over species field (but don't click)
  6. Use scroll wheel -> the field is scrollable when it shouldn't be

Platform Parity

Sync Offline Map When Online

Previously, I chose not to raise this platform disparity but i'd like to raise this requirement now.

I'd like to see the Profile view resemble in form and in function (with modifications made for desktop) to that of the iOS application and as it is specified. The reason why this specified design is an improvement to what is here (IMO) is because a user might want to synchronize or delete an offline map when they are operating in online work mode. Currently, a user much switch into offline mode in order to sync or delete the offline map. This presents an undue UX hurdle.

What this means WRT the design spec is that this view incoroprate items 5-8 and they adhere to the same layout and operate similarly (identical?) to how they operate in the iOS app.

Working Online, No Offline Map Working Online, Has Offline Map Working Offline
Screen Shot 2020-09-15 at 1 47 55 PM Screen Shot 2020-09-15 at 2 15 06 PM Screen Shot 2020-09-15 at 2 30 52 PM

This feature in its current form is acceptable because it solves the problem but I think there's room for improvement here and an opportunity to increase parity from platform to platform. If you think it better, we can extract this requirement from this PR and place it into its own issue. Do let me know what you want to do here, I can go either way.

Coordinate Portal Profile Info

There is a disparity in what Portal User information is displayed in the Profile view. Specifically, the iOS application displays the user's email and the .Net application displays the user's username. ArcGIS Collector aligns with .Net on this one. What do you think Data Collection should do?

For me what is most important is that there is parity.

iOS .Net
Screen Shot 2020-09-15 at 11 57 38 AM Screen Shot 2020-09-15 at 11 58 21 AM

Portal Profile Info Runtime Metadata

Currently the iOS and Android apps support displaying App & Runtime metadata, It appears the .Net app does not. Can we get this metadata added to the Profile view? In iOS, this information is derived by looking up information stored in the app's Bundle (not hardcoded).

I'm thinking this information would convey well just beneath the Sign In/ Sign Out button of the login pane.

Again, this request is inconsequential WRT this PR, if you find it more appropriate to do so we can move this request to a new issue.

Screen Shot 2020-09-15 at 2 34 50 PM

# Conflicts:
#	RELEASE.md
#	src/DataCollection.WPF/DataCollection.WPF (.NET Core).csproj
@nCastle1
Copy link
Collaborator Author

I've filed #103 and #104 to address feedback that is outside the scope of this PR. The other bug mentioned, about combobox scrolling, is actually how the built-in control is expected to behave. It can cause problems under parallels, due to inconsistencies between how macOS and Windows handle scrolling, especially with trackpads. I verified that the current behavior matches native UWP apps and ArcGIS Pro.

@esreli
Copy link
Contributor

esreli commented Sep 18, 2020

#99 (comment)

Sounds good, we'll prioritize these at a later date.

If the base branch is master then from an OSA release perspective, we're nearly there. The app version number has been incremented and the RELEASE notes are cut. It appears all that's left is updating the README and docs/README.

@nCastle1 nCastle1 changed the base branch from master to main September 18, 2020 21:00
Copy link
Contributor

@mikewilburn mikewilburn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I caught a few things and already spoke with Nathan about them, so I'll push up some commits which address them in a few moments.

Biggest thing that I'm undecided on is whether this release warrants a step up in the minor version and not just a patch.

docs/README.md Show resolved Hide resolved
docs/README.md Outdated Show resolved Hide resolved
docs/README.md Show resolved Hide resolved
RELEASE.md Outdated
Comment on lines 3 to 8
## 1.2.4

* Updates both WPF and UWP with a new design that adapts to work well for any screen size.
* Adds a `ModernMapPanel` custom layout panel to facilitate consistent, responsive design for UWP and WPF.
* Refactors styles and related XAML.
* Switches to vector icons where possible.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My gut says that because this release is pushing out such a significant overhaul to the UI, it's worth calling this version 1.3, but I know that diverges from the version numbering we had planned. @esreli @nCastle1 what do you guys think?

@nCastle1 nCastle1 changed the base branch from main to dev September 21, 2020 20:42
@nCastle1 nCastle1 merged commit f251d46 into dev Sep 21, 2020
@nCastle1 nCastle1 deleted the ncastle/panel-based-ui branch September 21, 2020 20:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants