Page MenuHomePhabricator

[web] introduce secondaryHeaderButton prop to Modal component
ClosedPublic

Authored by ginsu on Oct 12 2023, 12:52 AM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Sep 14, 5:01 PM
Unknown Object (File)
Sat, Sep 14, 5:01 PM
Unknown Object (File)
Sat, Sep 14, 5:01 PM
Unknown Object (File)
Sat, Sep 14, 5:01 PM
Unknown Object (File)
Sat, Sep 14, 5:01 PM
Unknown Object (File)
Sat, Sep 14, 4:59 PM
Unknown Object (File)
Fri, Sep 6, 9:26 PM
Unknown Object (File)
Sep 2 2024, 3:53 PM
Subscribers

Details

Summary

As a preliminary step to introducing the user profile menu component, we need to update the modal component. This diff makes two changes to the Modal component API

  1. Made the name prop optional
  2. introduce secondaryHeaderButton

Linear task: https://linear.app/comm/issue/ENG-5190/update-modal-component

Depends on D9447

Test Plan

Please see the screenshot where I pass in a UserProfileMenu component (will be introduced in a subsequent diff) as the secondaryHeaderButton

Screenshot 2023-10-12 at 3.56.54 AM.png (1×3 px, 938 KB)

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

Harbormaster returned this revision to the author for changes because remote builds failed.Oct 12 2023, 1:00 AM
Harbormaster failed remote builds in B23200: Diff 31952!
ginsu requested review of this revision.Oct 12 2023, 1:26 AM
atul added inline comments.
web/modals/modal.react.js
88–104 ↗(On Diff #31954)

Can memoize this guy

web/modals/user-profile/user-profile-modal.react.js
26 ↗(On Diff #31954)

Glad we could clean this up

This revision is now accepted and ready to land.Oct 16 2023, 9:32 AM

address atul's comments + rebase before landing