google / peoplemath

Web application for team planning
Apache License 2.0
127 stars 36 forks source link

Update reordering CSS and expand clickable area for objectives #154

Closed sttrinh closed 3 years ago

sttrinh commented 3 years ago

Bucket: Update reordering style and animation

Expand clickable area for objectives

sttrinh commented 3 years ago

Those 3 points you mentioned should all be pretty straightforward to fix. Since we are about 80% of the way there in terms of feature complete for this PR, it wouldn't take much more to fix the bugs you identified and add more tests to cover all our bases.

I agree with your assessment of the complexity though. As I was making the changes I realized I was making way more changes than I anticipated, and was not sure if I should keep going.

As you are the primary maintainer of the code, I think it all boils down to whether you would feel comfortable supporting this different way of event binding. If not, it's perfectly understandable and we don't have to merge this in order to avoid any potential consequences and we maintain consistency across the app. So I'm slightly leaning towards not merging this since it seems like a big tradeoff.

amdw commented 3 years ago

Many thanks! If the only benefit we get from this is a better clickable area on objectives and the grey highlighting, my sense is that the asking price is a bit too high and we should probably stop here. Is that OK with you?

Again, really appreciate the time you took to send this PR - the first commit is merged already.

sttrinh commented 3 years ago

Yup totally ok, thanks! I'll send more PRs later if I think of something