Skip to content

Show current organization's name and logo atop main menu #1937

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Show current organization's name and logo atop main menu #1937

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
87ddd72
store: Make `realmName` and `realmIcon` getters non-nullable
rajveermalviya Oct 22, 2025
c40a8d9
store [nfc]: Rename _tryResolveUrl to _tryResolveUrlStr
rajveermalviya Oct 28, 2025
83f3f9e
store: Add resolveRealmIconUrl
rajveermalviya Oct 28, 2025
c9c67d6
home: Show current organization's name and logo atop main menu
rajveermalviya Oct 22, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions lib/model/store.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -436,7 +436,7 @@ abstract class PerAccountStoreBase {
/// Resolve [reference] as a URL relative to [realmUrl].
///
/// This returns null if [reference] fails to parse as a URL.
Uri? tryResolveUrl(String reference) => _tryResolveUrl(realmUrl, reference);
Uri? tryResolveUrl(String reference) => _tryResolveUrlStr(realmUrl, reference);
Copy link
Collaborator

@chrisbobbe chrisbobbe Nov 4, 2025

Choose a reason for hiding this comment

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

store [nfc]: Rename _tryResolveUrl to _tryResolveUrlStr

flutter analyze is complaining to me at this commit:

Analyzing zulip-flutter...                                              

   info • Unnecessary 'this.' qualifier • lib/model/emoji.dart:229:25 • unnecessary_this
   info • Unnecessary 'this.' qualifier • lib/model/emoji.dart:234:26 • unnecessary_this

2 issues found. (ran in 3.5s)


/// Always equal to `connection.zulipFeatureLevel`
/// and `account.zulipFeatureLevel`.
Expand Down Expand Up @@ -465,10 +465,10 @@ abstract class PerAccountStoreBase {
int get selfUserId => core.selfUserId;
}

const _tryResolveUrl = tryResolveUrl;
const _tryResolveUrlStr = tryResolveUrlStr;

/// Like [Uri.resolve], but on failure return null instead of throwing.
Uri? tryResolveUrl(Uri baseUrl, String reference) {
Uri? tryResolveUrlStr(Uri baseUrl, String reference) {
Comment on lines 475 to +476
Copy link
Collaborator

@chrisbobbe chrisbobbe Nov 4, 2025

Choose a reason for hiding this comment

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

Can we move this helper to a different file, like maybe lib/basic.dart? It's a convenience function encapsulating some control flow (a try/catch) and doesn't depend on details of a store.

Ditto the new, parallel helper that you're adding, which takes a Uri instead of String for reference.

Copy link
Collaborator

@chrisbobbe chrisbobbe Nov 4, 2025

Choose a reason for hiding this comment

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

Or, alternatively, I think we could just make both of these helpers private.

try {
return baseUrl.resolve(reference);
} on FormatException {
Expand Down
2 changes: 1 addition & 1 deletion test/widgets/content_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -401,7 +401,7 @@ void main() {
// The image indeed has an invalid URL.
final expectedImages = (example.expectedNodes[0] as ImagePreviewNodeList).imagePreviews;
check(() => Uri.parse(expectedImages.single.srcUrl)).throws<void>();
check(tryResolveUrl(eg.realmUrl, expectedImages.single.srcUrl)).isNull();
check(tryResolveUrlStr(eg.realmUrl, expectedImages.single.srcUrl)).isNull();
Comment on lines 403 to +404
Copy link
Collaborator

@chrisbobbe chrisbobbe Nov 4, 2025

Choose a reason for hiding this comment

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

The rename brings attention to this existing check 🙂 which looks redundant with the one above it (.throws<void>()). I think we're free to remove it.

// The MessageImagePreview has shown up,
// but it doesn't attempt a RealmContentNetworkImage.
check(tester.widgetList(find.byType(MessageImagePreview))).isNotEmpty();
Expand Down