[native] [refactor] [ENG-530] rename inline-sidebar to inline-engagement
AbandonedPublic
Actions

Authored by • benschac on May 11 2022, 7:52 AM.

Details

Reviewers

• jacek
tomek
atul

Summary

rename component to match web component

Test Plan

flow / eslint should pass no new functionality. We're changing the name of this component platform wide to inline-engagement from inline-sidebar

Diff Detail

Repository

rCOMM Comm

Branch

inline-sidebar-web-and-native-ENG-530

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

• benschac created this revision.May 11 2022, 7:52 AM

Herald added subscribers: • abosh, • adrian, ashoat. · View Herald TranscriptMay 11 2022, 7:52 AM

I still need to update css class names, please ignore this diff for the moment.

• benschac added a parent revision: D4001: [native] [feat] [ENG-530] add new styles to inline sidebar component.May 11 2022, 7:53 AM

Harbormaster completed remote builds in B9009: Diff 12547.May 11 2022, 8:06 AM

• benschac requested review of this revision.May 11 2022, 8:06 AM

Thoughts on InlineMessageAction or something? We use "Action" instead of "Engagement" on web so it might be more consistent?

In D4005#112299, @atul wrote:

Thoughts on InlineMessageAction or something? We use "Action" instead of "Engagement" on web so it might be more consistent?

https://github.com/CommE2E/comm/blob/inline-sidebar-web-and-native-ENG-530/lib/hooks/inline-engagement-text.react.js#L16 Is what I'm referring to with engagement this is the diff where the suggestion is made https://phabricator.ashoat.com/D3895. Happy to make the change if you and @ashoat think it's a better name. IMO engagement seems like a good name.

atul accepted this revision.May 11 2022, 8:47 AM

This revision is now accepted and ready to land.May 11 2022, 8:47 AM

In D4005#112314, @benschac wrote:

In D4005#112299, @atul wrote:

Thoughts on InlineMessageAction or something? We use "Action" instead of "Engagement" on web so it might be more consistent?

https://github.com/CommE2E/comm/blob/inline-sidebar-web-and-native-ENG-530/lib/hooks/inline-engagement-text.react.js#L16 Is what I'm referring to with engagement this is the diff where the suggestion is made https://phabricator.ashoat.com/D3895. Happy to make the change if you and @ashoat think it's a better name. IMO engagement seems like a good name.

Thanks for linking the previous discussion

• benschac added a child revision: D4115: [native] [feat] [ENG-530] add icon to inline engagement.May 24 2022, 7:47 AM

(Copied from D3893)

@jacek I think we can abandon these changes since you've started a separate stack to deal with InlineSidebar. Let me know if I'm mistaken and I can re-open this if it tracks some unique work.

This revision now requires review to proceed.Aug 15 2022, 9:53 AM

atul abandoned this revision.Aug 15 2022, 9:53 AM

atul foisted this revision upon • benschac.

atul edited reviewers, added: atul; removed: • benschac.