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

Deprecate feature_state_counters #25884

Closed
Tracked by #25883
germain-gg opened this issue Jul 31, 2023 · 17 comments · Fixed by matrix-org/matrix-react-sdk#11343
Closed
Tracked by #25883

Deprecate feature_state_counters #25884

germain-gg opened this issue Jul 31, 2023 · 17 comments · Fixed by matrix-org/matrix-react-sdk#11343
Assignees
Labels
T-Task Tasks for the team like planning Z-Compound

Comments

@germain-gg
Copy link
Contributor

germain-gg commented Jul 31, 2023

Related to #25883

Screenshot 2023-07-31 at 11 28 36

The revamped version of the room header does not allocate space for the room counters, so we are planning to sunset that lab experiment.

@germain-gg germain-gg added the T-Task Tasks for the team like planning label Jul 31, 2023
@germain-gg germain-gg self-assigned this Jul 31, 2023
@nadonomy
Copy link
Contributor

nadonomy commented Jul 31, 2023

@germain-gg for reference, what's the maintenance burden like for this?

Thinking out loud, I'm wondering if now is the right time to to deprecate this and break peoples workflows. Another opportunity could be when we iterate on pinned messages (as, you could pin a message and continually edit it) which at the moment is a part of 2024 planning.

@t3chguy
Copy link
Member

t3chguy commented Jul 31, 2023

cc @erikjohnston as it was your labs flag

@germain-gg
Copy link
Contributor Author

@nadonomy there's no maintenance burden, it has not been touched in years. We can keep this if we iterate on the designs that were provided. But the idea is to apply to simplification philosophy we had previously agreed on.

I don't really think the pinned messages would change anything to the outcome. This feature exists so those counters get updated by a CI bot.
In my opinion this code needs to be pulled from Element Web, and if we really think there is value we can add a module API customisation point to insert a custom UI to display those counters.

@daniellekirkwood
Copy link
Contributor

+1 to the module api solution

Personally, I really like the feature but as we're simplifying our UI and starting to build the strategy for the modules work, this feels like it falls solidly into modules rather than core functionality/features.

@erikjohnston
Copy link
Member

I'd be very sad to see this go, as its an incredibly useful feature (at least for the Backend team, I can't stress how useful it is to have important information embedded into the team room). Ideally if anything I'd like to take this out of labs, though I appreciate that that adds a burden on design / product to make the UI better.

(More generally as a side bar: I'm quite sad that we don't really make any use of the power of Matrix with custom events to provide a much richer experience / integration with people's work flows. Look at e.g. Slack for how powerful that can be, while we currently limit ourselves to just formatted text).

@erikjohnston
Copy link
Member

RE module API: How would that work in practice? Does that mean to use the simple counters you'd need to use a particular element web deployment?

@t3chguy
Copy link
Member

t3chguy commented Jul 31, 2023

It is also very useful for the Web App Team to have the pending reviews and release blockers surfaced in a very visible place. Pinning wouldn't work as an alternative as it'd need manually checking, at which point why not just manually check a website. Module API wouldn't either as developers want to be running develop/Nightly which won't include modules. An alternative would be a chatty bot which continually reminds the room, plaguing the timeline. Another alternative would be the Markdown topics lab but it seems like topics are also leaving the room header...

RE module API: How would that work in practice? Does that mean to use the simple counters you'd need to use a particular element web deployment?

Yes, modules needs to be included at build time

@daniellekirkwood
Copy link
Contributor

The objective we've been given is to simplify the product and make it easier to use for all.

My 2 cents: I agree that this feature is especially useful to our internal workflows but it doesn't match our product strategy either. If we can't find a way to make the modules work, then our option is to remove it. If we're not simplifying the product then we're not adhering to the strategy that we've laid out - making an exception for this case would mean we make exceptions for other cases and that's not reducing complexity or removing the problem we're tackling today.

@erikjohnston
Copy link
Member

@daniellekirkwood That's broadly fair. There are a couple of points I'd like to add though (but this is very much me advocating from the outside, so don't feel you have to engage particularly as I don't want to be a massive drag):

  • I can see why removing the feature simplifies the code base (and that's a perfect valid argument by itself), I'm not sold on the fact that removing this feature simplifies the product for users (other than we shouldn't be hiding useful features behind labs flags unnecessarily). In particular, this is mostly an opt-in feature; in that rooms have to set up an integration to encounter it. Are we explicitly trying to remove working but under used features of the app?
    • FTR I'm happy to try and take on the maintenance burden of fixing it if changes break it.
  • I agree that we shouldn't be biasing product decisions based on our internal workflows, though the flip side of that is of course "if its useful to us its likely useful to others", eg is there a world in which this helps make our ESS offering more compelling by increasing the usefulness/snazziness of the integrations?
  • Does this exemplify a gap in our strategy? Removing hooks from element web that allow users/developers/orgs to take advantage of the structure of Matrix feels like a step backwards (though admittedly at the moment we're in the worst of all worlds were we sort of support it but nobody really knows its there or how to use it).
    • Is the module API the right way of exposing those sort of hooks if in practice nobody would use modules? Longer term is a different system something that should be investigated?
    • I know the lack of movement on extensible events has effectively blocked us from exploring this space further, but I expect we would need to have a vague idea of how we want them to work; i.e. how would you support richer event formats such as Github web hooks and the like? Would the rendering of them be part of the core element web offering or would you need to build your own and pull in a suitable module?

I think I see this as almost two separate issues: 1) what do we do about this particular feature, and 2) how would we support other similar features in the future (and do we want to)?

Again, I don't want to block the team from making progress by shouting from the peanut gallery, but I'd also be sad if we removed the feature if its not actually harming the product/team.

@nadonomy
Copy link
Contributor

nadonomy commented Aug 1, 2023

@erikjohnston the thinking and input is really helpful, and no you're absolutely not being a drag! High temperature stuff from me:

Are we explicitly trying to remove working but under used features of the app?

Looking at this specific feature in isolation, I agree with you that removing it seems hasty. Our broader goals are to:

  1. Upgrade Element Web to meet modern accessibility standards. To avoid regressions, we need to basically re-build the entire front-end, with modern best practises.
  2. Simplify surface area to aid the above, and reduce debt (in every way you can interpret the word debt).
  3. Remove things which don't have a validated, widespread use.

I've been trying to balance if this is something we should maintain, and looking through the priorities above I don't think it qualifies for the burden moving forward.

FTR I'm happy to try and take on the maintenance burden of fixing it if changes break it.

This is really kind - but given it doesn't have widespread use at the moment I'm not sure we can justify spending time to 'fix it' enough to be easily maintainable in the first place. We'd need to be convinced it's a differentiator, it's discoverable, harmonious with other features, and so on.

I agree that we shouldn't be biasing product decisions based on our internal workflows, though the flip side of that is of course "if its useful to us its likely useful to others", eg is there a world in which this helps make our ESS offering more compelling by increasing the usefulness/snazziness of the integrations?

Does this exemplify a gap in our strategy? Removing hooks from element web that allow users/developers/orgs to take advantage of the structure of Matrix feels like a step backwards (though admittedly at the moment we're in the worst of all worlds were we sort of support it but nobody really knows its there or how to use it).

In the future for sure would love to solve this. But, the utility is limited to developers. I'd stack rank the importance of this behind making the common extensibility cases work first - bots, bridges, widgets easy for anyone to set up and maintain.

Look at e.g. Slack for how powerful that can be, while we currently limit ourselves to just formatted text).

Agree! The major missing pieces where Slack succeed in their foundations are Block Kit to ensure a good end experience, and the very sane limitations Slack impose - they know when to constrain and force you to jump out to a full screen experience in your browser.

So on balance, I think we should remove the feature. I know it's a shame to break the workflows, but I think it's important we prioritise making Element Web accessible and reduce surface area to help with that goal.

I find it difficult to get behind the Module API thinking tbh - that's a great escape hatch for web developers who are able and invested to develop their own modules and maintain their own deployments - but it excludes everyone else. What we're talking about here is often updated glanceable information. I'd rather instead we find the limits of things like:

  • For simple strings, counters: Bots updating room topics, messages and/or pinned messages in rooms
  • For anything more complex: Creating/embedding widgets

With this approach, even as a non-developer I've been able to set up really powerful integrations with e.g. off the shelf webhook bots.

@t3chguy
Copy link
Member

t3chguy commented Aug 1, 2023

For simple strings, counters: Bots updating room topics, messages and/or pinned messages in rooms

It looks like the topic won't be shown unless you hover so this would be too easy to miss

@nadonomy
Copy link
Contributor

nadonomy commented Aug 1, 2023

For simple strings, counters: Bots updating room topics, messages and/or pinned messages in rooms

It looks like the topic won't be shown unless you hover so this would be too easy to miss

Our confidence isn't 100% on this. We're going to:

  1. Show it momentarily, before tweening it out.
  2. Test this first on develop to learn, gather feedback.
  3. Be on standby to consider just always showing it depending on what we learn from the above.

@erikjohnston
Copy link
Member

Thanks @nadonomy. That makes sense to me, it's a shame but the conclusion is fairly cut and dry.

In the future for sure would love to solve this. But, the utility is limited to developers.

To slightly disagree with this: It won't help non-developers in a personal messenger setting, but I think it would help in professional environments where there are devs around to help create the integrations (which is somewhat how it works inside Element).

@nadonomy
Copy link
Contributor

nadonomy commented Aug 1, 2023

To slightly disagree with this

I agree with you. 😀 It just falls out of the immediate focus we're trying to solve for.

@daniellekirkwood
Copy link
Contributor

I really appreciate the time and thought that's gone into making this an open and supportive discussion. Unfortunately where we've landed appears to be: Removing the feature.

@germain-gg I believe your PR is still in draft state, do you need any further info or help in moving it on?

@daniellekirkwood
Copy link
Contributor

FWIW I've posted this in a few internal rooms so that folks aren't surprised when it lands.

@germain-gg
Copy link
Contributor Author

@daniellekirkwood I have everything i need to move this forward.
As discussed internally, this will be removed at the same time we delete the legacy room header code.

@germain-gg germain-gg removed their assignment Oct 12, 2023
@kerryarchibald kerryarchibald self-assigned this Nov 15, 2023
Johennes added a commit to matrix-org/matrix-react-sdk that referenced this issue Nov 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-Task Tasks for the team like planning Z-Compound
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants