Page MenuHomePhabricator
Differential D6158

[web] Add expand buttons
ClosedPublic
Actions

Authored by inka on Jan 4 2023, 3:50 AM.
Tags
None
Referenced Files
F3356950: D6158.diff
Sat, Nov 23, 9:39 PM
Unknown Object (File)
Tue, Nov 19, 9:17 PM
Unknown Object (File)
Tue, Nov 19, 6:54 PM
Unknown Object (File)
Fri, Nov 8, 10:16 PM
Unknown Object (File)
Mon, Nov 4, 11:07 AM
Unknown Object (File)
Mon, Oct 28, 8:29 AM
Unknown Object (File)
Oct 17 2024, 2:52 PM
Unknown Object (File)
Oct 17 2024, 1:31 AM
View All Files
Subscribers
ashoat
atul
tomek

Details

Reviewers
• przemek
michal
ginsu
tomek
Commits
rCOMMdb464b3a31bf: [web] Add expand buttons
Summary

Add buttons to be used in community drawer to expand items.
Related issue
The same buttons on native : D5718

Test Plan

Displayed buttons in AppSwitcher, checked that they display as expected and onClick works

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

inka created this revision.Jan 4 2023, 3:50 AM
Herald added subscribers: atul, tomek, ashoat. · View Herald TranscriptJan 4 2023, 3:50 AM
Harbormaster completed remote builds in B15073: Diff 20564.Jan 4 2023, 4:05 AM
inka requested review of this revision.Jan 4 2023, 4:05 AM
inka edited the summary of this revision. (Show Details)Jan 4 2023, 5:09 AM
inka edited the summary of this revision. (Show Details)Jan 4 2023, 5:18 AM
michal added a comment.Jan 4 2023, 5:22 AM

Is there a reason why we don't just have a disabled property in props?

web/sidebar/expand-buttons.react.js
20–26 ↗(On Diff #20564)
.buttonWrapper {
  font-size: 12px;
  width: 24px;
  height: 24px;
}

Could this be simplified to something like this?

inka added a child revision: D6160: [web] Add drawer item implementations.Jan 4 2023, 5:56 AM
• przemek added inline comments.Jan 5 2023, 4:18 AM
web/sidebar/expand-buttons.react.js
12 ↗(On Diff #20564)

We should use mixed. See: https://phab.comm.dev/D6047?id=20388#inline-41131

30–36 ↗(On Diff #20564)

You can pass disabled props to ExpandButton and then to Button and create conditional style based on it if needed.

• przemek requested changes to this revision.Jan 5 2023, 4:19 AM
This revision now requires changes to proceed.Jan 5 2023, 4:19 AM
inka updated this revision to Diff 20673.Jan 9 2023, 5:32 AM

Address code review, simplify

Harbormaster completed remote builds in B15162: Diff 20673.Jan 9 2023, 5:47 AM
• przemek accepted this revision.Jan 10 2023, 12:36 AM
This revision is now accepted and ready to land.Jan 10 2023, 12:36 AM
inka added a reviewer: tomek.Jan 10 2023, 2:20 AM
This revision now requires review to proceed.Jan 10 2023, 2:20 AM
tomek accepted this revision.Jan 10 2023, 10:27 AM
tomek added inline comments.
web/sidebar/expand-buttons.css
10 ↗(On Diff #20673)

Please use a constant from typography.css.

This revision is now accepted and ready to land.Jan 10 2023, 10:27 AM
inka updated this revision to Diff 20791.Jan 11 2023, 5:48 AM

Use values from typography.css

Harbormaster completed remote builds in B15251: Diff 20791.Jan 11 2023, 6:02 AM
inka updated this revision to Diff 22435.Feb 13 2023, 7:10 AM

Rebase

Harbormaster failed remote builds in B16398: Diff 22435!Feb 13 2023, 7:23 AM
inka updated this revision to Diff 22465.Feb 13 2023, 7:57 AM

Add .js to imports

Harbormaster completed remote builds in B16428: Diff 22465.Feb 13 2023, 9:23 AM
inka updated this revision to Diff 22523.Feb 14 2023, 2:26 AM

Rebase

Harbormaster completed remote builds in B16476: Diff 22523.Feb 14 2023, 2:41 AM
Closed by commit rCOMMdb464b3a31bf: [web] Add expand buttons (authored by • InkaSokolowska). · Explain WhyFeb 14 2023, 5:01 AM
This revision was automatically updated to reflect the committed changes.
• InkaSokolowska added a commit: rCOMMdb464b3a31bf: [web] Add expand buttons.

Revision Contents
Changeset List

PathSize
web/
sidebar/
13 lines
33 lines
2 lines

Diff 22465

View Options

web/sidebar/expand-buttons.css

Loading...
View Options

web/sidebar/expand-buttons.react.js

Loading...
View Options

web/theme.css

Loading...