stream settings: Prep for radio-button UI, to handle more than two privacy options by chrisbobbe · Pull Request #5360 · zulip/zulip-mobile

chrisbobbe · 2022-04-26T21:04:58Z

I've got a draft for an input component that lets the user choose from more than two options, radio-button style, for #5250. These are some small improvements I've noticed while working on that draft.

chrisbobbe · 2022-04-26T21:06:22Z

+          // TODO: Group all the stream's attributes together (name,
+          //   description, policies, etc.), with an associated "Edit"
+          //   button that gives a UI for changing those attributes. For the
+          //   grouping, try react-native-paper's `Card`, with a
+          //   ZulipTextButton in its `Card.Actions`. See
+          //     https://callstack.github.io/react-native-paper/card-actions.html
+          //   Or their `Surface`:
+          //     https://callstack.github.io/react-native-paper/surface.html


It seemed like it might take a while to get react-native-paper set up just right, so this stuff isn't in my plan for getting #5250 out the door. But I wanted to write it down.

alya · 2022-04-27T05:08:59Z

If it would be helpful to review the UI, a screenshot would be great. :)

gnprice · 2022-04-27T21:57:54Z

Thanks! These code changes all look good.

I see there are some small UI changes. I think it's a good practice for us to try to always have at least a quick review of UI changes too -- so either a screenshot posted, or look together in the office. Let's perhaps do the latter this afternoon.

chrisbobbe · 2022-04-28T18:21:02Z

Sure, makes sense! I'll post screenshots in two separate comments, coming up.

chrisbobbe · 2022-04-28T18:48:43Z

abe083a

SelectableOptionRow: Use our standard icon size of 24 for the checkmark

439865c

SelectableOptionRow: Reserve a space for the checkbox even when unselected

So that the word wrapping of the subtitle, when long, doesn't change
when `selected` changes. That'd be distracting.

A list of SelectableOptionRow components before these changes, then after, with the same title/subtitle in both cases (but longer than I think we've yet used in the app). We currently use SelectableOptionRow in the language picker and the set-user-status screen.

Apr-28-2022.11-38-55.mp4

Apr-28-2022.11-43-10.mp4

chrisbobbe · 2022-04-28T18:52:48Z

fef1ae7

StreamSettingsScreen: Make it a bit clearer what the "Edit" button does

And leave a TODO for something better.

The screen currently gives a jumbled collection of

- attributes about the stream and a button to change them

- a subscribe/unsubscribe button, and, if the user is subscribed,
  details about the subscription and switches to change the details

- a way to change who else is subscribed (but only by adding
  subscribers, and with no way to see the state before or after a
  change)

Each of these bullet points should be given its own coherent section
in the UI. Probably we should experiment with react-native-paper's
`Card`, as mentioned in the comment. This would be similar to what
Slack on iOS does.

Before/after (it just changes the button's text from "Edit" to "Edit stream"):

alya · 2022-05-04T18:20:39Z

The wording change in the stream settings looks good to me. For a bigger reorg in the future, we should think about lining it up better with the organization/terminology in the web UI:

alya · 2022-05-04T18:23:11Z

I'm having a hard time visualizing precisely how status and language picker will be affected, but having the checkmark be a standard size and reserving the space for it sounds great.

chrisbobbe · 2022-05-04T19:19:20Z

I'm having a hard time visualizing precisely how status and language picker will be affected

Good point, I forgot to screenshot those! Here we are:

Status suggestions before/after:

The "reserve-space" change means that all rows have enough height for the checkmark, not just the row(s) with a checkmark. That's another way the layout won't change distractingly when a row's selected state changes. I guess this change becomes noticeable with the icon's size increase from 16 to 24, for rows that have just one line of text, with the standard font size.

Language picker before/after:

gnprice

Catching up after my vacation. One code comment on the new revision, below. Now reading the thread above…

gnprice · 2022-05-10T01:40:18Z


      try {
-        await api.createStream(auth, { name, description, invite_only });
+        api.createStream(auth, { name, description, invite_only: privacy === 'private' });


This lost the await, which means the try/catch will have no effect. Perhaps unintentional from the rebase?

Yep, thanks for the catch!

gnprice · 2022-05-10T01:42:04Z

OK, all looks good modulo that code comment. Thanks for the screenshots!

…ected So that the word wrapping of the subtitle, when long, doesn't change when `selected` changes. That'd be distracting.

And leave a TODO for something better. The screen currently gives a jumbled collection of - attributes about the stream and a button to change them - a subscribe/unsubscribe button, and, if the user is subscribed, details about the subscription and switches to change the details - a way to change who else is subscribed (but only by adding subscribers, and with no way to see the state before or after a change) Each of these bullet points should be given its own coherent section in the UI. Probably we should experiment with react-native-paper's `Card`, as mentioned in the comment. This would be similar to what Slack on iOS does.

We'd like to add more stream-privacy options soon.

chrisbobbe · 2022-05-10T04:14:00Z

Thanks for the review! Merged.

chrisbobbe added the P1 high-priority label Apr 26, 2022

chrisbobbe requested a review from gnprice April 26, 2022 21:04

chrisbobbe commented Apr 26, 2022

View reviewed changes

chrisbobbe force-pushed the pr-stream-settings-prep branch from 58fe7b9 to 13c584b Compare April 27, 2022 01:39

chrisbobbe force-pushed the pr-stream-settings-prep branch from 13c584b to 02b603c Compare April 28, 2022 18:53

chrisbobbe mentioned this pull request Apr 28, 2022

StreamSettingsScreen: s/Private/Privacy/; use radio buttons, not a switch #5369

Merged

chrisbobbe force-pushed the pr-stream-settings-prep branch from 02b603c to 062815d Compare May 4, 2022 20:15

gnprice reviewed May 10, 2022

View reviewed changes

chrisbobbe added 9 commits May 9, 2022 21:08

EditStreamCard [nfc]: Convert to a function component

57c9813

SelectableOptionRow [nfc]: Define styles closer to where it's used

aa8bbb9

SelectableOptionRow: Reserve a space for the checkbox even when unsel…

fbda98f

…ected So that the word wrapping of the subtitle, when long, doesn't change when `selected` changes. That'd be distracting.

SelectableOptionRow: Use our standard icon size of 24 for the checkmark

7da3fc3

SelectableOptionRow [nfc]: Name some style identifiers a bit better

1220631

EmojiPickerScreen [nfc]: Fix a typo in a comment

0183018

streams types [nfc]: Let Flow infer some types

c93db3a

EditStreamCard [nfc]: Model stream-privacy as an enum, not boolean

f82defe

We'd like to add more stream-privacy options soon.

chrisbobbe force-pushed the pr-stream-settings-prep branch from 062815d to f82defe Compare May 10, 2022 04:13

chrisbobbe merged commit f82defe into zulip:main May 10, 2022

chrisbobbe deleted the pr-stream-settings-prep branch May 10, 2022 04:14

Uh oh!

Conversation

chrisbobbe commented Apr 26, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chrisbobbe Apr 26, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alya commented Apr 27, 2022

Uh oh!

gnprice commented Apr 27, 2022

Uh oh!

chrisbobbe commented Apr 28, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chrisbobbe commented Apr 28, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chrisbobbe commented Apr 28, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alya commented May 4, 2022

Uh oh!

alya commented May 4, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

chrisbobbe commented May 4, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gnprice left a comment

Choose a reason for hiding this comment

Uh oh!

gnprice May 10, 2022

Choose a reason for hiding this comment

Uh oh!

chrisbobbe May 10, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gnprice commented May 10, 2022

Uh oh!

chrisbobbe commented May 10, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

chrisbobbe commented Apr 26, 2022 •

edited

Loading

chrisbobbe Apr 26, 2022 •

edited

Loading

chrisbobbe commented Apr 28, 2022 •

edited

Loading

chrisbobbe commented Apr 28, 2022 •

edited

Loading

chrisbobbe commented Apr 28, 2022 •

edited

Loading

alya commented May 4, 2022 •

edited

Loading

chrisbobbe commented May 4, 2022 •

edited

Loading

chrisbobbe May 10, 2022 •

edited

Loading