Add exploding icon for chat input. #12009

adamjspooner · 2018-05-23T18:26:39Z

@keybase/react-hackers, this is ready for review.

adamjspooner · 2018-05-23T18:27:13Z

shared/chat/conversation/input-area/normal/platform-input.desktop.js

+              this.state.explodingPopupOpen && (
+                <SetExplodingMessagePopup
+                  attachTo={this.props.attachmentRef}
+                  isNew={true}


@buoyad, how should this be determined?

Not yet implemented, I haven't gotten to it yet in the integration

adamjspooner · 2018-05-23T18:47:46Z

Actually, I need to get mobile into this PR. So, please hold off on reviewing. 🙏

codecov · 2018-05-23T19:09:15Z

Codecov Report

Merging #12009 into master will decrease coverage by 0.05%.
The diff coverage is 94.73%.

@@            Coverage Diff             @@
##           master   #12009      +/-   ##
==========================================
- Coverage   87.32%   87.27%   -0.06%     
==========================================
  Files        1262     1198      -64     
  Lines       66274    66001     -273     
==========================================
- Hits        57872    57600     -272     
+ Misses       8402     8401       -1

Impacted Files	Coverage Δ
...hared/chat/conversation/input-area/normal/types.js	`100% <ø> (ø)`	⬆️
...ared/chat/conversation/input-area/normal/shared.js	`100% <100%> (ø)`
...sation/input-area/normal/platform-input.desktop.js	`98.92% <100%> (+0.11%)`	⬆️
...sation/messages/set-explode-popup/index.stories.js	`81.25% <100%> (+1.25%)`	⬆️
...rsation/messages/set-explode-popup/index.native.js	`100% <100%> (ø)`	⬆️
...d/chat/conversation/input-area/normal/container.js	`76.71% <75%> (+0.52%)`	⬆️
shared/common-adapters/meta.js	`95.34% <90%> (+1.06%)`	⬆️
...at/conversation/input-area/normal/index.stories.js	`83.49% <94.28%> (+5.11%)`	⬆️
...rsation/input-area/normal/platform-input.native.js	`96.15% <97.91%> (+0.42%)`	⬆️
shared/actions/platform-specific.desktop.js	`85.1% <0%> (ø)`	⬆️
... and 87 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4e74e60...a373583. Read the comment docs.

adamjspooner · 2018-05-24T13:04:19Z

@keybase/react-hackers, this is ready for review.

adamjspooner · 2018-05-24T13:48:42Z

Just noticed I've left something out of mobile: https://zpl.io/beg7PBY

buoyad

Looks good, a couple changes

buoyad · 2018-05-24T15:15:02Z

shared/chat/conversation/input-area/normal/platform-input.desktop.js

    super(props)
    this.state = {
      emojiPickerOpen: false,
+      explodingPopupOpen: false,


This comes in through the FloatingMenuParentHOC too, as props.showingMenu and props.toggleShowingMenu()

buoyad · 2018-05-24T15:17:57Z

shared/chat/conversation/input-area/normal/platform-input.desktop.js

@@ -279,6 +292,36 @@ class PlatformInput extends Component<PlatformInputProps, State> {
                  type="iconfont-boom"
                />
              )}
+            {flags.explodingMessagesEnabled &&
+              this.state.explodingPopupOpen && (
+                <SetExplodingMessagePopup


This is close to needing to be factored into a connected component. But it can probably wait until the isNew flag integration

buoyad · 2018-05-24T15:20:25Z

shared/chat/conversation/input-area/normal/platform-input.native.js

@@ -29,6 +35,14 @@ class PlatformInput extends Component<PlatformInputProps, State> {
    this.props.inputSetRef(ref)
  }

+  _explodingPickerToggle = () => {


same here, this and state.explodingPickerOpen can go away to use the props from the HOC

buoyad · 2018-05-24T15:22:21Z

shared/chat/conversation/input-area/normal/platform-input.native.js

+      <ExplodingIcon
+        explodingModeSeconds={explodingModeSeconds}
+        isExploding={isExploding}
+        openExplodingPicker={openExplodingPicker}


On native on device this isn't dismissing the keyboard, so there's no way to make a selection without backing out of the convo, going back in, and choosing before focusing the input.

Found a solution with a little fiddling, changing _explodingPickerToggle to focus or blur the input depending on whether the popup is open works.

Dismissing the keyboard feels pretty weird. Is there a way to attach the picker so that it stays adjacent to the keyboard instead of under it?

I've done this within the adapter. Thoughts?

Sure, that seems ok.

buoyad · 2018-05-24T15:28:17Z

shared/chat/conversation/input-area/normal/shared.js

+  let title = 'New'
+
+  if (explodingModeSeconds !== 0) {
+    const description = messageExplodeDescriptions.find(


I don't think this should look for a match in messageExplodeDescriptions. If it's set to something different than one of the choices, it doesn't show anything. Instead, this could do a couple checks on whether explodingModeSeconds is > a day, an hour, etc. and create a ballpark meta based on that. There are some similar functions in #12024 for displaying the countdown. Actually, formatTimeDifference could be a drop-in replacement for this, it just takes in milliseconds instead of seconds.

That's a good idea. I commented on formatTimeDifference in your PR.

I'll wait for #12024 to merged so I can use that helper.

buoyad · 2018-05-24T15:32:21Z

shared/common-adapters/meta.js

  color?: string,
  backgroundColor: string,
+  lowercase?: boolean,


might be better as noUppercase so callers can set mixed case stuff freely

buoyad · 2018-05-24T15:33:33Z

shared/common-adapters/text.meta.desktop.js

@@ -238,6 +238,19 @@ const metaData: {[key: TextType]: MetaType} = {
    fontSize: 11,
    styleOverride: globalStyles.fontRegular,
  },
+  // Body tiny
+  BodyTiny: {


We needed a smaller text style to match the design. Is this not the way to do it?

In those cases I usually use the closest TextType and apply the necessary styles adhoc. I think these are meant to correspond with what's in the font styleguide

adamjspooner · 2018-05-25T21:54:46Z

@keybase/react-hackers, this is ready for another review. I ran into a known bug with overflow on Android: facebook/react-native#17074, and I'm still working through it (as seen in the last commit). This is behind a feature flag. So, we should be okay to merge it.

buoyad

lgtm pending native flag

buoyad · 2018-05-25T22:00:35Z

shared/chat/conversation/input-area/normal/platform-input.native.js

@@ -166,6 +208,11 @@ const Action = ({hasText, onSubmit, isEditing, openFilePicker, insertMentionMark
    </Box>
  ) : (
    <Box2 direction="horizontal" gap="small" style={styles.actionIconsContainer}>
+      <ExplodingIcon


feature flag needed here too

Add exploding icon for chat input.

353f9ab

adamjspooner commented May 23, 2018

View reviewed changes

adamjspooner added 2 commits May 24, 2018 09:01

Add exploding icon to mobile.

6410b5f

Hello, Storyshots.

9830bf0

buoyad suggested changes May 24, 2018

View reviewed changes

adamjspooner added 11 commits May 24, 2018 14:05

Merge master.

f432f53

Use the HOC's props.

9b33134

Review feedback.

1639af5

Hide the keyboard on mobile.

540ba3e

Remove unused font style.

9925c28

Merge branch 'master' into spooner/DESKTOP-6731-exploding-icon

f5fb8fc

Use util.

6f696e9

Merge branch 'master' into spooner/DESKTOP-6731-exploding-icon

b50816e

Merge branch 'master' into spooner/DESKTOP-6731-exploding-icon

15133da

Dismiss the keyboard on native.

bc2734a

Start fixing Android's implementation.

f108c7f

buoyad approved these changes May 25, 2018

View reviewed changes

Wrap in a feature flag.

a373583

adamjspooner merged commit f60d259 into master May 30, 2018

adamjspooner deleted the spooner/DESKTOP-6731-exploding-icon branch June 5, 2018 13:05

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add exploding icon for chat input. #12009

Add exploding icon for chat input. #12009

adamjspooner commented May 23, 2018

adamjspooner May 23, 2018

buoyad May 23, 2018

adamjspooner commented May 23, 2018

codecov bot commented May 23, 2018 •

edited

Loading

adamjspooner commented May 24, 2018

adamjspooner commented May 24, 2018

buoyad left a comment

buoyad May 24, 2018

buoyad May 24, 2018

buoyad May 24, 2018

buoyad May 24, 2018

buoyad May 24, 2018

adamjspooner May 24, 2018

adamjspooner May 24, 2018

buoyad May 24, 2018

buoyad May 24, 2018

adamjspooner May 24, 2018 •

edited

Loading

adamjspooner May 24, 2018

buoyad May 24, 2018

adamjspooner May 24, 2018

buoyad May 24, 2018

adamjspooner May 24, 2018

buoyad May 24, 2018

adamjspooner commented May 25, 2018

buoyad left a comment

buoyad May 25, 2018

Add exploding icon for chat input. #12009

Add exploding icon for chat input. #12009

Conversation

adamjspooner commented May 23, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adamjspooner commented May 23, 2018

codecov bot commented May 23, 2018 • edited Loading

Codecov Report

adamjspooner commented May 24, 2018

adamjspooner commented May 24, 2018

buoyad left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adamjspooner May 24, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adamjspooner commented May 25, 2018

buoyad left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented May 23, 2018 •

edited

Loading

adamjspooner May 24, 2018 •

edited

Loading