Topic hierarchy follow up

[cstns]

Description

• restructured hierarchy computed prop to extend functionality
• remember open state for each topic segment
• alter topic count to include deeply nested topics not just immediate children
• add the ability to copy topic path

Related Issue(s)

closes #4801
closes #4802
also implemented a topic open state management mentioned in the main PR as a comment

Checklist

• I have read the contribution guidelines
• Suitable unit/system level tests have been added and they pass
• Documentation has been updated
  • Upgrade instructions
  • Configuration details
  • Concepts
• Changes flowforge.yml?
  • Issue/PR raised on FlowFuse/helm to update ConfigMap Template
  • Issue/PR raised on FlowFuse/CloudProject to update values for Staging/Production

Labels

• Includes a DB migration? -> add the area:migration label

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.70%. Comparing base (ac51a56) to head (e925f84).
Report is 24 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #4818   +/-   ##
=======================================
  Coverage   78.70%   78.70%           
=======================================
  Files         316      316           
  Lines       15152    15152           
  Branches     3486     3486           
=======================================
  Hits        11925    11925           
  Misses       3227     3227           

Flag	Coverage Δ
backend	78.70% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[Steve-Mcl]

Serban, the topic count at the fully collapsed level is 8 in this screenshot but it is not reflective of the actual number of topics. You are counting the number of topics + the number of levels.

Let me try to clarify:
There really only is 5 topics:

• another-long-topic with spaces and Not-well-formed/at/all/really/test1
• another-long-topic with spaces and Not-well-formed/at/all/really/test2
• another-long-topic with spaces and Not-well-formed/at/all/really/test3
• another-long-topic with spaces and Not-well-formed/at/all/really/test6
• another-long-topic with spaces and Not-well-formed/at/all/really/test8
  ... but your tree is counting nesting levels as additional topics

In MQTT Explorer, this is clearer as they only show the count on the immediate parent:

[cstns]

Altered the counting method to only consider terminating segments/children

This implies that a response of

'foo/bar',
'foo/bar/paz',
'foo/bar/baz',
'foo/bar/asd',
'foo/faz',
'foo/taz'

will yield

Not 100% sure if the initial foo/bar should be counted off as a terminating segment as it has nested topics. @hardillb please advise

[hardillb]

Yes if foo/bar has a message

[cstns]

In this case, the UI would need some tweaking to let users know that foo/bar is a standalone topic otherwise it would seem that the counter is glitching

@joepavitt I'd appreciate your input on the UI side

[cstns]

@hardillb would something along these lines be suggestive enough?

[Steve-Mcl]

@hardillb would something along these lines be suggestive enough?

Cant we just lose the slashes in the visualisation (obviously they are still required to make a valid topic in the copy data) and where there is an empty topic, perhaps a grey (empty) placeholder would be better?

Also. that count of 7 doesn't look right? What were the underlying topics that built that out?

[hardillb]

Removing the slashes works, but would still need a way to indicate mid topic message.

The video from Steve has access to the last message value which helps, but we don't have that.

How about not adding the extra entry (with the bare /) but adding a * to the actual row in the tree

e.g.

foo 
      | - bar *
              | - asd
              | - baz
              | - paz 

[cstns]

Also. that count of 7 doesn't look right? What were the underlying topics that built that out?

These were just some images I took of the hierarchy after manipulating the dom to figure out a way to represent the different states the counter can be ignored for the moment until we decide on how we represent the data.

Cant we just lose the slashes in the visualisation (obviously they are still required to make a valid topic in the copy data) and where there is an empty topic, perhaps a grey (empty) placeholder would be better?

How about not adding the extra entry (with the bare /) but adding a * to the actual row in the tree

This would be a iteration with empty placeholders and * to represent mid topic endpoints (maybe also putting a title on the * with helper text to make it more suggestive)

generated from the following payload:

[
    '/flim/flam', // empty start segment
    'bork//spork', // empty middle segment
    'blim/bizzle/squark/blurp',
    'blim/bizzle/squark/florp/', // empty end segment
    'foo/bar', // middle terminating segment with nested topics
    'foo/bar/paz',
    'foo/bar/baz',
    'foo/bar/asd',
    'foo/faz',
    'foo/taz'
]

I think the ^ payload includes all possible use cases

[Steve-Mcl]

A * is a valid character in a topic so presenting it where one does not exist feels wrong.

Also, I really think we should not show the topic separators / - they are implied by the tree structure. This is how the very popular MQTT explorer does it and deviation from that may cause confusion.

[cstns]

This is the latest iteration:

• removed slashes from topic headers (except empty terminating topics - see florp)
• counters reflect the topics count (including terminating topics which are included in the parent count not on itself - see foo which includes bar, which is a terminating topic)
• currently marking terminating topics with a heroicon archive icon which when hovered has a help cursor and 'This topic is also able to receive events' title which is displayed when hovered over

[joepavitt]

currently marking terminating topics with a heroicon archive icon which when hovered has a help cursor and 'This topic is also able to receive events' title which is displayed when hovered over

Can you explain what this means please? I'd expect every tier to be able to receive topics? The archive icon looks like a delete/remove icon

[hardillb]

currently marking terminating topics with a heroicon archive icon which when hovered has a help cursor and 'This topic is also able to receive events' title which is displayed when hovered over

Can you explain what this means please? I'd expect every tier to be able to receive topics? The archive icon looks like a delete/remove icon

While every level can, not every level has, the marker is on the topics that have had a message published, leaving intermediate levels unmarked.

[cstns]

From what I can tell not all topics have messages. If i have a topic response with the following form:

[
   'acme/foo', 
    'acme/foo/bar/paz',
    'acme/foo/bar/baz',
]

only the terminating foo, paz and baz topics hold messages while acme and bar don't.

We don't display topic messages like MQTT Explorer does so we'd need a way to demarcate what topics hold or don't hold messages. While terminating topics are self explanatory that they have messages, topics like foo are not represented/demarcated accordingly in our interface.

MQTT Explorer displays the last received message even on middle topics and for all terminating topics so you can differentiate between topic segments that are holding messages.

Another representation solution that was mentioned was to mark middle topics with {...} or we could go with another hero-icon.

[knolleary]

Herewith my opinions. In summary, lets keep things simple. Lets not worry about markers in the topic hierachy where messages are published or not. We can revisit that if/when we consider enabling a live view of content.

Empty topic at the start

At the top level, rather than (empty), lets display it as /flim - ie, show the leading / (which we otherwise don't show).

Topic counts

Lets just use the count of nodes in the graph below the level - regardless of leaf/edge/message/no-message etc.

So given

[
   'acme/foo', 
    'acme/foo/bar/paz',
    'acme/foo/bar/baz',
]

• acme will display a count of 4
• acme/foo will show 3
• acme/foo/bar will show 2

[joepavitt]

Agree with everything @knolleary has said

[cstns]

At the top level, rather than (empty), lets display it as /flim - ie, show the leading / (which we otherwise don't show).

The problem with this is that '/' or '(empty)' as we represent it is a valid root topic.

As for the counter and not demarcating segments with messages, I can go wherever which way you decide.

[cstns]

The problem with this is that '/' or '(empty)' as we represent it is a valid root topic.

missed the leading slash in /flim.

Ok then, I'll adjust accordingly!

[knolleary]

@cstns super close, but one case not quite working as expected.

If I publish to top/a/b - then it correctly shows me top at the top level:

If I then publish to /top/c/d - it then only shows /top at the top level, and the topics under top have now been placed under it:

In this scenario, I'd expect to see separate top-level entries for top and /top

[cstns]

should be fixed now, I don't know how i missed it 😅

[knolleary]

Looks good to me!