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

Document feature_notifications_panel labs flag #25924

Merged
merged 3 commits into from
Sep 20, 2023

Conversation

germain-gg
Copy link
Contributor

@germain-gg germain-gg commented Aug 4, 2023

For https://github.com/vector-im/element-internal/issues/444

Checklist

  • Tests written for new code (and old code if feasible)
  • Linter and other CI checks pass
  • Sign-off given on the changes (see CONTRIBUTING.md)

This change is marked as an internal change (Task), so will not be included in the changelog.

@germain-gg germain-gg requested a review from a team as a code owner August 4, 2023 10:44
docs/labs.md Outdated

## Notifications panels (`feature_notifications_panel`)

A list of all events notifying the current user. Only works with unencrypted content.
Copy link
Member

Choose a reason for hiding this comment

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

Only works with unencrypted content.

This isn't true, it depends on a user's push rules. If a user marked their .m.room.encrypted* push rules with highlight=true then it'd include those too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But in reality, does that ever happen? I'd be more inclined to add smething a bit simpler

Copy link
Member

Choose a reason for hiding this comment

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

Its also not right because we don't only show notifications - we show only highlighting notifications. You will have more notifications than you will find in this panel even for unencrypted rooms. notify != highlight

Copy link
Member

Choose a reason for hiding this comment

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

In reality if we wish to maintain the highlight behaviour this panel should probably be renamed to Mentions

@germain-gg
Copy link
Contributor Author

@daniellekirkwood could you help us resolve the wording discussion above?

I'm not convinced we should rebrand this piece of UI at this stage, nor should we go with a too technical description. It might be a good idea to revamp it the day we put some more attention and care to it.

@daniellekirkwood
Copy link
Contributor

Labs flags in product appear to have the structure: Header, subhead so my suggestion would be this:

Enable the notifications panel in the room header
Unreliable in encrypted rooms

@germain-gg germain-gg requested a review from t3chguy September 1, 2023 10:05
@germain-gg germain-gg merged commit dcc0f9f into develop Sep 20, 2023
25 of 26 checks passed
@germain-gg germain-gg deleted the germain-gg/labs/notifications branch September 20, 2023 07:21
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants