Discussion: inconsistent Entity Id label display in the UI

[mikejed]

Earlier this week, after working with eddyb on using EntityCommands with Metrics, we filed an enhancement request ( #2611 ) to display MetricId in the MetricDetail block. Jon jumped on that and got it wrapped up very quickly.

Once I saw that method and played a little bit with adding the same label to Groups on my dev system, I thought I'd make a survey of which Detail type blocks would benefit from a similar treatment.

Here's what I found:

The following blocks already showed the ID, even though the ID is already provided in the URL of the page they're on:

• Finance\BatchDetail (this used what I'll call a "non-standard" color)
• Reporting\DataViewDetail (this was the only one I noticed that didn't hide the Id: 0 label when creating a new one)
• WorkFlow\WorkflowTypeDetail

The following blocks already showed the ID, because the Page Parameters (url) do not expose it:

• Reporting\MetricDetail (this was added by Jon yesterday)

The following blocks do not appear to show the ID, even though they're on pages where the Page Parameters (url) do not expose it:

• Follow\SuggestionDetail (eventId is used instead)
• Groups\GroupAttendanceDetail (GroupId, Date and Schedule Id is used instead)
• Security\RestKeyDetail (restUserId is used instead)

I was unable to find pages with these blocks installed, or didn't have the experience/setup to check the display of the following blocks:

• Cms\HtmlContentDetail
• Cms\LayoutDetail
• Connection\ConnectionOpportunityDetail
• Connection\ConnectionRequestDetail
• Connection\ConnectionTypeDetail
• Core\RestActionDetail
• Core\SignatureDocumentDetail
• Core\SignatureDocumentTemplateDetail
• Prayer\PrayerCommentDetail
• Security\AccountDetail
• Utility\StarkDetail

All other display type blocks show the EntityId in the Page Parameters (url), and so they don't have the Id label in the detail block title.

Discussion:

So, here's the discussion I'm interested in: should the Id label be added to all blocks which don't show them?

Pros:

• It's a consistent and reliable experience for users to find the Id of the entity they're viewing. Cons:
• Some entities already show a number of labels, and it's possible it would over-clutter the interface in some cases

OR

Should we cherry-pick a few "common" entities whose detail blocks display the ID: those entities which newcomers to EntityCommands would likely be dealing with?

Groups, ContentChannel, ContentChannelItem, ContentChannelType, Site, Campus, DefinedType, Location, Calendar, EventItem, EventItemOccurrence, Financial Account, Financial Transaction, Group, GroupMember, GroupType, and Report (to match DataView) would be my first draft list of detail blocks which might benefit from the label.

Pros:

• It covers a majority of entities which beginners would use, without having to rigerously add it to every. single. detail block. Cons:
• The "common" entities are also those already loaded with labels, so there's still the possibility of cluttering them up.

OR

Is there something to be said for removing the label from WorkflowType, DataView, and Batch so that the only entities which receive the Id label are those which aren't identified in the Page Parameters

OR do we just leave it alone and add the label to the blocks as we identify a need?

I've already added a branch to my fork that hides the Id label on the DynamicDataDetail block when you're creating a new Dynamic Data entity, and if some guidance or consensus could be gained from this discussion, I'd be happy to finish editing within that branch and submit the PR.

[jonedmiston]

Thanks for bringing this up. I struggle with this myself (not knowing the Id). I figure it was only me and I usually have a SQL window open so I turn to that. I think we could sweep through the detail blocks (I don't really like to have to tell someone to look in the URL as that seems a bit technical for some). What if we added it to the 'panel drawer' (shown below) and removed it from the header.

[mikejed]

I think that's a great solution- it kind of ticks all the boxes at once :+1:

I also meant to say that I would like to conduct a similar survey of the List blocks, as these frequently launch modals on entities- modals which sometimes include Id labels, (such as on Defined Values), and which sometimes don't. I'd love to see that made consistent as well.

Let me know if I should add that data here or open up another discussion topic for that.

[jonedmiston]

So you'd recommend consistency on having the Id on the modal (just want to confirm my understanding).

[mikejed]

I would, yes

[cabal95]

Just noting that the panel drawer solution wouldn’t work for workflow activity types - though I like the solution for other uses.

[jonedmiston]

@cabal95 right, and you already added them there.

So the results are:

1. Sweep through all blocks ending in ...Detail.ascx and add the Id to the panel drawer if the Id != 0 (not in add mode).
2. Sweep through all blocks ending in List.ascx. If they have a modal then add the Id to the modal header like the Page Properties modal when editing page settings on the every page.

Does anyone see any issues?

[mikejed]

Sounds right to me. I'll delete my branch which fixes the DataViewDetail Id label displaying when a new one is being created, since that label is getting moved anyway. If you'd like me to take this off your plate, perhaps you can provide me a pattern to follow in each of these block types and I can go through and make these changes and create a PR. Or if it's really fast or easier to just do it yourself, I'm ok with that too. Just trying not to add to your loads

[jonedmiston]

Well @nairdo ruined all our fun. He wrote the panel drawer in such a way that it only took a couple of lines of code to the drawer class to add the id to every block that uses the drawer. When software is written right hard things are easy. https://github.com/SparkDevNetwork/Rock/commit/44e0e0654c8fb403da460641106cc29c184ec7de

I'll work on a pattern for you to follow for the modals.

[jonedmiston]

@mikejed I'm looking for a modal that needs an Id and having a hard time finding one. Can you suggest one for me to use as a template?

[jonedmiston]

I updated the Id to show the model name too. Thoughts? In some places (especially the calendar) it's easy to get confused with which model the detail page is for.

[mikejed]

@edmistj I missed your request yesterday. The Defined Value modal that pops up from the Defined Type lists are a good example of the Id showing up. http://rock.rocksolidchurchdemo.com/page/119?definedTypeId=30

On the other hand, the Edit Role modal that pops up on Group Types does not have the Id, and that would be extremely useful, I can tell you from experience. http://rock.rocksolidchurchdemo.com/page/117?groupTypeId=1

Related: the tooltips on the DefinedValueList block can be problematic- it could be worth discussing if they should just be eliminated now that this will be a standard UI throughout Rock

[jonedmiston]

@mikejed YES, that tooltip messes with me once a week. Funny what I'll put up with myself but have no problem fixing for others. I fixed that and here is a template for change a modal to have the id. Note we'd only want to show this when editing as it will have the value of 0 when adding.

https://github.com/SparkDevNetwork/Rock/commit/667a35a490f722d944b66c5347e6288f5efa0792

I'm going to close this item now that I think we have a plan going forward.

[NuclearTacos]

I think that there may have been a regression on this feature.

Patch Notes

Prealpha

Prealpha

[nairdo]

yeah, that's odd. I don't recall ever seeing that code (https://github.com/SparkDevNetwork/Rock/commit/44e0e0654c8fb403da460641106cc29c184ec7de) in there. Where did it go... There's no history of it either: https://github.com/SparkDevNetwork/Rock/commits/develop/Rock/Web/UI/Controls/PanelDrawer.cs