Paths

Table of Contentst

Differential D3667

[web] Convert `Modal` into functional component
ClosedPublic
Actions

Authored by • jacek on Apr 8 2022, 4:24 AM.

Tags

None

Referenced Files

	F3610836: D3667.id11465.diff
	Wed, Jan 1, 12:34 AM

	F3610652: D3667.id11877.diff
	Wed, Jan 1, 12:18 AM

	F3610218: D3667.id11922.diff
	Tue, Dec 31, 11:09 PM

	F3610078: D3667.id11933.diff
	Tue, Dec 31, 10:50 PM

	Unknown Object (File)
	Sun, Dec 29, 3:23 AM

	Unknown Object (File)
	Sat, Dec 14, 7:18 AM

	Unknown Object (File)
	Fri, Dec 6, 4:49 PM

	Unknown Object (File)
	Tue, Dec 3, 4:45 AM

Subscribers

Details

Reviewers

tomek
atul
• benschac

Commits

rCOMM2986754c24d3: [web] Convert `Modal` into functional component

Summary

Convert exiting Modal component from class into functional component to simplify the logic and make future changes easier. No logic changes has been made.

Test Plan

All existing modals should work as before

Diff Detail

Repository

Branch

jacek/modal-3

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

• jacek created this revision.Apr 8 2022, 4:24 AM

Herald added subscribers: • abosh, • benschac, • karol and 4 others. · View Herald TranscriptApr 8 2022, 4:24 AM

Harbormaster completed remote builds in B8019: Diff 11224.Apr 8 2022, 4:30 AM

• jacek requested review of this revision.Apr 8 2022, 4:30 AM

• jacek added a child revision: D3669: [web] Redesign `Modal` to match Figma.Apr 8 2022, 6:01 AM

• jacek added a parent revision: D3666: [web] Center modals vertically.

• jacek added reviewers: tomek, atul, • benschac.Apr 8 2022, 6:06 AM

Looks good!

web/modals/modal.react.js
18	Setting the default when destructuring looks a lot cleaner than the old `defaultProps` approach.
32	Outside of the scope of this diff, but might be good to add a comment noting that `keyCode` 27 corresponds to the `ESC` key? I had a hunch, but still had to google to double check.

This revision is now accepted and ready to land.Apr 8 2022, 10:38 AM

• jacek added inline comments.Apr 10 2022, 11:40 PM

web/modals/modal.react.js
32	Good idea. I was thinking about adding the comment (as I also had to use https://keycode.info/ to check), but I forgot about it.

rebase

Harbormaster completed remote builds in B8194: Diff 11465.Apr 14 2022, 3:06 AM

rebase

Harbormaster completed remote builds in B8518: Diff 11877.Apr 25 2022, 10:35 AM

rebase

Harbormaster completed remote builds in B8553: Diff 11922.Apr 26 2022, 8:41 AM

Closed by commit rCOMM2986754c24d3: [web] Convert `Modal` into functional component (authored by • jacek). · Explain WhyApr 26 2022, 9:09 AM

This revision was automatically updated to reflect the committed changes.

• jacek added a commit: rCOMM2986754c24d3: [web] Convert `Modal` into functional component.

Revision Contents
Changeset List

Path

Size

web/

modals/

126 lines

Diff 11224

web/modals/modal.react.js

Loading...