Skip to content

Add a banner to encrypted rooms with visible history. #4738

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.

Sign up for GitHub

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

Jump to bottom
Merged
pixlwave merged 12 commits into from
Dec 11, 2025
Merged

Add a banner to encrypted rooms with visible history. #4738

pixlwave merged 12 commits into from
Dec 11, 2025

Conversation

@kaylendog
Copy link
Contributor

@kaylendog kaylendog commented Nov 12, 2025 •
edited
Loading

  • Adds a dismissable alert that is displayed whenever the user opens a room with history_visibility != joined. When cleared, this is recorded in the app's data store.
  • When opening a room with history_visibility = joined, this flag is cleared.

Issue: element-hq/element-meta#2875

Pull Request Checklist

UI changes have been tested with:

  • iPhone and iPad simulators in portrait and landscape orientations.
  • Dark mode enabled and disabled.
  • Various sizes of dynamic type.
  • Voiceover enabled - I don't believe I can do this from the simulator.

@kaylendog kaylendog self-assigned this Nov 12, 2025
@CLAassistant
Copy link

CLAassistant commented Nov 12, 2025 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

@kaylendog kaylendog added the pr-feature for a new feature label Nov 12, 2025
@kaylendog kaylendog mentioned this pull request Nov 12, 2025
Open
3 tasks
@kaylendog kaylendog marked this pull request as ready for review December 8, 2025 14:46
@kaylendog kaylendog requested a review from a team as a code owner December 8, 2025 14:46
@kaylendog kaylendog requested review from stefanceriu and removed request for a team December 8, 2025 14:46
@kaylendog
feat: Add history visible alert.
c68aba4 
- Adds a dismissable alert that is displayed whenever the user
  opens a room with `history_visibility` != `joined`. When cleared,
  this is recorded in the app's data store.
- When opening a room with `history_visibility` = `joined`, this
  flag is cleared.

Issue: element-hq/element-meta#2875
@kaylendog
tests: Add unit tests for history sharing in RoomScreenFooterView.
6286d45
@kaylendog kaylendog force-pushed the kaylendog/history-sharing/alert branch from ee76a1a to 6286d45 Compare December 8, 2025 15:08
@pixlwave pixlwave requested review from pixlwave and removed request for stefanceriu December 10, 2025 15:38
pixlwave
pixlwave requested changes Dec 10, 2025
View reviewed changes
Copy link
Member

@pixlwave pixlwave left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Very nice, thanks @kaylendog! Comments are mainly just minor Swift style preferences but we do need to include the translated string before merging this :)

Also worth noting that we all run Xcode configured like so, so that new lines retain their respective indentation:

Screenshot 2025-12-10 at 4 11 46 pm

ElementX/Sources/Application/Settings/AppSettings.swift Outdated

case elementCallBaseURLOverride

case acknowledgedHistoryVisibleRooms
Copy link
Member

@pixlwave pixlwave Dec 10, 2025

Choose a reason for hiding this comment

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

Super minor nit: Could we put this under hasSeenNewSoundBanner so its position in the keys kind of reflects its position in the settings.

Copy link
Contributor Author

@kaylendog kaylendog Dec 11, 2025

Choose a reason for hiding this comment

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

Ah! I did not see these, I think it would be nice to rename this to hasSeenHistoryVisibleBannerRooms. Addressed in c59a88d.

Copy link
Member

@pixlwave pixlwave Dec 11, 2025

Choose a reason for hiding this comment

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

Usually we would prefix with is/has/can when the value is a Bool, so similarly to seenInvites I think this should drop the has. If I'm honest, I think that acknowledgedHistoryVisibleRooms was probably the clearest name for this anyway 🙃

Copy link
Contributor Author

@kaylendog kaylendog Dec 11, 2025

Choose a reason for hiding this comment

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

Eeep, I shall revert!

@pixlwave
Copy link
Member

pixlwave commented Dec 10, 2025

Voiceover enabled - I don't believe I can do this from the simulator.

FWIW, a close approximation is to use Xcode | Open Developer Tool | Accessibility Inspector and check the labels in the simulator that way, but seeing as this is based on existing views I wouldn't worry about it too much :)

@kaylendog kaylendog requested a review from pixlwave December 11, 2025 12:50
pixlwave
pixlwave approved these changes Dec 11, 2025
View reviewed changes
ElementX/Sources/Application/Settings/AppSettings.swift Outdated

case elementCallBaseURLOverride

case acknowledgedHistoryVisibleRooms
Copy link
Member

@pixlwave pixlwave Dec 11, 2025

Choose a reason for hiding this comment

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

Usually we would prefix with is/has/can when the value is a Bool, so similarly to seenInvites I think this should drop the has. If I'm honest, I think that acknowledgedHistoryVisibleRooms was probably the clearest name for this anyway 🙃

@sonarqubecloud
Copy link

sonarqubecloud bot commented Dec 11, 2025

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@pixlwave pixlwave changed the title Add alert to encrypted rooms with visible history (iOS). Add a banner to encrypted rooms with visible history. Dec 11, 2025
@pixlwave pixlwave merged commit aa525a5 into element-hq:develop Dec 11, 2025
6 of 7 checks passed
pixlwave added a commit that referenced this pull request Dec 12, 2025
@pixlwave
Revert "Add a banner to encrypted rooms with visible history. (#4738)"
072e7f5 
This reverts commit aa525a5.
pixlwave added a commit that referenced this pull request Dec 12, 2025
@pixlwave
Revert "Add a banner to encrypted rooms with visible history. (#4738)" (
2eb02cd 
#4850)

This reverts commit aa525a5.
@pixlwave pixlwave mentioned this pull request Dec 12, 2025
Draft
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@pixlwave pixlwave pixlwave approved these changes

Assignees

@kaylendog kaylendog

Labels

pr-feature for a new feature

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@kaylendog @CLAassistant @pixlwave