Display tags as sorting items in the community panel

[elinorbgr]

This PR adds the basics or room tagging functionality, for #80.

Currently, it only adds a tags field to RoomInfo containing the list of associated tags, discarding any metadata they convey. I would like some feedback about whether this is the correct route for implementing this state tracking before inserting more elaborate logic in there.

[mujx]

👍

This seems like the correct path, saving the necessary info in cache between sync requests.

[Valodim]

Providing motivation: "Pinning" of rooms the way Riot does with Favorites (which uses room tags) is the one thing I'm missing from nheko to use it as a daily driver these days. Very excited for this feature :)

[elinorbgr]

I tried rebasing this before continuing to advance, and the compiler now complains with:

.../nheko/src/Cache.cpp: In member function ‘void Cache::saveState(const mtx::responses::Sync&)’:
.../nheko/src/Cache.cpp:872:29: error: ‘mpark’ has not been declared
                         if (mpark::holds_alternative<
                             ^~~~~

Has something been changed recently regarding the way dependencies are handled... ?

edit: Ah, nevermind, mtxclient switched to using boost's variant.

[mujx]

Yes the mpark library has been removed, the API is slightly different.

[elinorbgr]

Okay, here is basic tag support. A generic "tag" icon is used for tags in the community tab.

I think that, for a minimally ergonomic UI, this needs to be added:

• display tag name as a tooltip on pointer overlay
• use custom icons for m.favourite and m.lowpriority
• ensure special ordering for tags and communities
  • world remains first
  • m.favourite is second
  • communities and tags are here, in an order to be decided
  • m.lowpriority is last

[elinorbgr]

I have setup basic icons for favourite and lowpriority rooms. They are probably not the best possible, but improving them is just a matter of changing the PNG files, and I guess this wouldn't block this PR?

Regarding the tooltip, I've gone for the convention:

• All rooms / Favourite rooms / Low priority rooms for world/m.favourite/m.lowpriority
• <tag name> (tag) for tags
• <community name> (community) for communities

Does that sound good?

Regarding the last point, I don't know how it is possible to manually order the elements in the vertical bar. Any hints somebody?

[mujx]

I've just tested the favourite & low priority tags and it works great! Not sure how we can test custom tags. Is there any other way than manually making the API calls?

The icons are not a blocker as they could be easily changed if something better comes up.

Sorting will have to be done manually (removing & re-positioning the widgets in the layout) as far as I know. There is something similar here.

[anoadragon453]

Allowing for creation of room tags doesn't seem that tricky, and would be a major win over Riot :)

[elinorbgr]

Is there any other way than manually making the API calls?

Unfortunately, not afaik. I'm also in favour of adding such an interface to nheko, but I'll reserve that for a future PR if that's okay.

For now, it's possible to add a tag to a room with the following curl command:

curl -X PUT -d '{"order": "1"}' "https://<homeserver>/_matrix/client/r0/user/<your-username>/rooms/<the-room-id>/tags/<chosen-tag>?access_token=<access_token>"

You need to fill in:

• <homeserver> : your homeserver domain
• <your-username> : your full username (@login:homeserver.tld)
• <the-room-id> : the id (starting with !) of the room to tag, aliases do not work
• <chosen-tag> : the tag to add to the room. Spec conventions user tags to be any utf8 string prefixed by u..
• <access_token> : an access token

[elinorbgr]

Woops, looks like QSharedPointer::get is too new for the Qt version of the CI, I'll replace that with QSharedPointer::data.

[mujx]

Thanks again for your work.

I've highlighted some small refactorings

[mujx]

Maybe add a member function or a lambda for those calls.

[elinorbgr]

Are you referring to the fact that contentsLayout_->insertWidget(contentsLayout_->count() - 1, item); is a long call and that it implies a lot of repetition?

I realized I could simply remove the final "Stretch" item and insert it back in the layout at the end, rather than insert all widgets "one before the end". Would that be okay too?

[mujx]

Yes, I was referring to that repetition. I'm fine with any solution that doesn't have a lot of duplication.

[mujx]

It would be nice to have this 4 number derived from the tag: string somehow. Currently it seems like a magic number if you only read a part of the code.

[elinorbgr]

Is adding a comment explaining it enough, or should I replace it by strlen("tag:")?

[mujx]

I've seen it in 2-3 places so maybe it would be better to use something like strlen.

[Valodim]

(with apologies for commenting from the sidelines:) If you're going to do custom room tags in the future, please write up a brief concept document and think through the user workflows first, and ideally sync up with Riot and their possible plans. It doesn't seem tricky because the technical bits are easy, but "going with the code flow" on these kinds of features is a slippery slope towards a fragmented ecosystem and bad user experience.

Thanks for your work either way, really looking forward to the release that includes this!

[elinorbgr]

@Valodim Regarding that, I had already pushed a few month ago for a better specification of the semantics of tags in the client-server spec: https://matrix.org/docs/spec/client_server/r0.4.0.html#id154, I believe this already sets a good basis for relative uniformity, do you think more is needed?

[mujx]

@Valodim Not sure what you mean by code flow?

Custom tags are supported by the spec so we're just taking advantage of an otherwise under-appreciated feature.

To elaborate a little more, this feature will allow arbitrary grouping of rooms under a namespace (i.e tag). Currently only displaying them is supported but in the near future you'll be able to create, delete & updated them, pretty much how Riot handles the default tags like favourites etc.

[elinorbgr]

@mujx I believe this last commit should address your comments.

[Valodim]

Oh, I didn't know there were namespaces for tags yet! Yeah that should work nicely, and great to hear there is initiative wrt spec work, too.

Relating to my previous comment, this seems like a use case that other clients may want to support. A vendor-neutral namespace with well defined semantics for the workflows you want to cover would facilitate that.

Perhaps the u. namespace would be sufficient for a straightforward first iteration, actually :)