Add "help" icon so users can find their Zulip server URL; fix some button styles#2320
Open
chrisbobbe wants to merge 11 commits into
Open
Add "help" icon so users can find their Zulip server URL; fix some button styles#2320chrisbobbe wants to merge 11 commits into
chrisbobbe wants to merge 11 commits into
Conversation
Saves churn on neighboring lines when a dartdoc changes. The convention comes from review feedback elsewhere: zulip#2279 (comment)
Before this, we were accidentally doing touch feedback in two overlapping ways: - the state-based colors from the Figma, which caused the color to change correctly but with no animation. - an animated color change with a translucent overlay, because we didn't cancel out the Material-default overlayColor by passing Colors.transparent for that. The animation is nice, but the color was chosen from Material defaults (as a transformation of the foreground/label color), and this meant accidentally deviating from the Figma. Instead, just do it one way: with overlayColor, so that we keep the animation. But use some arithmetic to make it match the Figma colors.
These cases previously used `someDesignVariable.withAlpha(0)`, keeping the variable's RGB while zeroing the alpha. But the result's RGB never matters: the static background paints as fully transparent, and `_overlayFor` only reads `base.a`, never `base.rgb` (its returned color carries `target`'s RGB). So `Colors.transparent` is equivalent.
I made this by copying /web/icons/help.svg, then iterating with Claude to harmonize it with our `info` icon, matching the size and stroke width of the outer circle. I'd be happy to have this replaced by something from a human designer, but I expect Vlad probably doesn't have the bandwidth right now.
This will mean much less duplicated code when we reorient ZulipIconButton around the icon-button component in the Figma's Zulip Web UI Kit.
This commit is NFC because it turns out that ZulipIconButton doesn't actually have any callers anymore! See 4b42f7a, where the last (possibly only ever?) caller was removed. It was a three-dots button in the "All channels" page that we removed because things looked cluttered. So we're free to change this without disrupting existing UI. There are a few callers in tests, which end up changing just a bit; I've made the `tooltip` param required, which helps with accessibility because the tooltip text propagates to a Semantics label. This new version is based on an "icon button" component in the Zulip Web UI Kit, which sits next to the "Button" component on the same Figma frame, which backs our ZulipWebUiKitButton widget. The two components use the same color palette, so it's natural and convenient for ZulipIconButton to share coloring code with ZulipWebUiKitButton. Also, expand the dartdoc of `baseFilledInputDecoration` with a good use case for this widget, which we'll demonstrate next.
chrisbobbe
added
maintainer review
|
Visual changes look good to me! Has there been a discussion about the "This is the address you use to open Zulip in a browser" string? Not sure if that can be clarified further, and whether it's worth showing a prefix-less "zulip.example.org" placeholder--though I'm guessing that prefix probably gets stripped off? |
Collaborator
Author
|
Ah no, that string hasn't been discussed anywhere; definitely open to suggestions. I'm not sure what prefix you're referring to, but we do already offer the hint text "your-org.zulipchat.com" when you've focused the input but not typed anything yet (you can try this yourself in the released app): 
In my screenshots in the PR description, I had typed "chat.zulip.org" into the input. |
Fixes zulip#109. Done with an icon button, rather than an inline link in the help text (as I think we'd envisioned at first), because that could be an annoyingly small touch target, and right next to the "Continue" button so easy to mis-tap.
chrisbobbe
force-pushed
the
pr-server-url-help-and-icon-button
branch
from
a0f026a to
2b29aec
Compare
Collaborator
Author
|
Updated the helper text:
|
rajveermalviya
approved these changes
May 19, 2026
Member
rajveermalviya
left a comment
rajveermalviya
left a comment
There was a problem hiding this comment.
Thanks @chrisbobbe! All LGTM, just one nit below.
|
|
||
| /// Make an [Icon] for an [IconData], | ||
| /// uniformly for [icon] and (when specified) [selectedIcon]. | ||
| Icon _buildIcon(IconData data) => Icon(data); |
Member
There was a problem hiding this comment.
nit: Is this helper method set up to expand in future, otherwise currently it just emits Icon() which could just be inlined?
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #109.
Notable commit message:
The question-mark icon opens https://zulip.com/help/logging-in#find-the-zulip-log-in-url and its tooltip is "Help".
Other button styles touched here: