Page MenuHomePhabricator

[web] remove input styles from style.css
AbandonedPublic

Authored by benschac on Feb 25 2022, 11:28 AM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Jan 6, 11:42 PM
Unknown Object (File)
Mon, Jan 6, 11:41 PM
Unknown Object (File)
Mon, Jan 6, 11:34 PM
Unknown Object (File)
Sat, Jan 4, 3:28 AM
Unknown Object (File)
Sat, Jan 4, 3:28 AM
Unknown Object (File)
Sat, Jan 4, 3:28 AM
Unknown Object (File)
Fri, Dec 27, 8:22 AM
Unknown Object (File)
Fri, Dec 27, 8:22 AM

Details

Reviewers
atul
ashoat
Summary

styles are in Input component and are no longer needed.

Test Plan

render Input component. They should look the same

Diff Detail

Repository
rCOMM Comm
Branch
fix-border-radius
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

atul requested changes to this revision.Mar 2 2022, 8:27 AM

I think this diff and D3292 should be combined?

This revision now requires changes to proceed.Mar 2 2022, 8:27 AM
In D3293#89380, @atul wrote:

I think this diff and D3292 should be combined?

Okay, historically I was told do one thing in a diff. That one thing is move or delete. Does combining a diff like this make sense @ashoat? If so could we document it somewhere?

Personally I would keep these diffs separate. They seem logically separate to me. Here are some questions you can ask to determine if two changes should be combined into the same diff:

  1. "Is it more mental overhead two think of these two changes together, or to think of them separately?"
  2. "Can I come up with a clear and concise diff title for each of these two changes? Can I come up with a clear and concise diff title for the two changes combined?"
atul accepted this revision.EditedMar 3 2022, 8:38 AM

Eh I disagree but it's fine..

The combined diffs would still be a single move operation.. but instead we have a partial move followed by another diff that completes the move

"Is it more mental overhead two think of these two changes together, or to think of them separately?"

More mental overhead to make the connection between two separate diffs than to see it all in one place.

This revision is now accepted and ready to land.Mar 3 2022, 8:38 AM
This revision now requires review to proceed.Mar 3 2022, 8:44 AM
This revision is now accepted and ready to land.Mar 3 2022, 10:20 PM