search

influxdata / clockface

UI Kit for building Chronograf
https://influxdata.github.io/clockface
MIT License
43 stars 18 forks source link

feat: Add Collapse functionality to Draggable Resizer #814

Closed brandenTenbrink closed 1 year ago

brandenTenbrink commented 1 year ago

Closes #

Changes

Added the ability to make any panel collapsible through a button. This functionality can be added as a isCollapsible prop on the DraggablePanel.

Screenshots

Screen Shot 2022-08-09 at 12 24 59 PM

Checklist

Check all that apply

brandenTenbrink commented 1 year ago

@ChitlangeSahas @hoorayimhelping I did an update to the remove the 0 and 1 terms from the variables. I'd also like to add that this component is slated for a code refactor--so as long as things are passible, I would like to get this deployed since it is a long over due functionality for QX

hoorayimhelping commented 1 year ago

I'd also like to add that this component is slated for a code refactor--so as long as things are passible, I would like to get this deployed since it is a long over due functionality for QX

About our response to this:

  1. The Dumplings team have repeatedly found ourselves refactoring code other teams have authored quickly when the ownership was passed over to us. We're often told "this code will be refactored soon," but it doesn't end up being refactored by the team that wrote it, and we have to take time out of what we're doing to handle basic things that should have been done as part of the initial authoring of the code. We've grown a bit sensitive to that and are tightening up what we consider passible accordingly.
  2. If this is a POC meant to be iterated on for a specific team's features, the changes almost certainly shouldn't be getting made in Clockface. Clockface is the generic component library for our UI (and anyone else who wants to use it in their UI). If there is specific functionality for specific features, that work should go in UI until they're generic enough to be used by multiple features on the UI, at which point they can get moved to Clockface and released. While in the U I, the standards of what is acceptable can be relaxed based on context, if necessary. This is how we handled Subway Navigation in the First Mile and Cloud Subscription features.
  3. The information that this is functionality that a team is waiting on and that it will be iterated on would be very helpful to know at the start of reviewing this PR. I think this is one of the best uses of a well-written PR description that provides context into the work being done. A link to a github issue, and/or a couple of sentences explaing the background of the work are very much appreciated in these instances.
brandenTenbrink commented 1 year ago

I'd also like to add that this component is slated for a code refactor--so as long as things are passible, I would like to get this deployed since it is a long over due functionality for QX

About our response to this:

  1. The Dumplings team have repeatedly found ourselves refactoring code other teams have authored quickly when the ownership was passed over to us. We're often told "this code will be refactored soon," but it doesn't end up being refactored by the team that wrote it, and we have to take time out of what we're doing to handle basic things that should have been done as part of the initial authoring of the code. We've grown a bit sensitive to that and are tightening up what we consider passible accordingly.
  2. If this is a POC meant to be iterated on for a specific team's features, the changes almost certainly shouldn't be getting made in Clockface. Clockface is the generic component library for our UI (and anyone else who wants to use it in their UI). If there is specific functionality for specific features, that work should go in UI until they're generic enough to be used by multiple features on the UI, at which point they can get moved to Clockface and released. While in the U I, the standards of what is acceptable can be relaxed based on context, if necessary. This is how we handled Subway Navigation in the First Mile and Cloud Subscription features.
  3. The information that this is functionality that a team is waiting on and that it will be iterated on would be very helpful to know at the start of reviewing this PR. I think this is one of the best uses of a well-written PR description that provides context into the work being done. A link to a github issue, and/or a couple of sentences explaing the background of the work are very much appreciated in these instances.
  1. Hence, the improvements suggested were made to the code.
  2. This is a general functionality that exists in many draggable resizers--the ability to collapse. It just so happens that this was also a functionality that was needed for QX.
  3. I agree with this point entirely. I can start doing that moving forward. It was a decision made later in the process--after realizing the struggle it was to simply understand how to work with the code of this component. The next time this component is worked on will reworking its code such that it remains the same from a implementation/prop perspective, but is much simpler to understand and modify moving forward: https://github.com/influxdata/clockface/issues/818

That being said, the component will be in a good spot for all use cases now that it was collapsible functionality.