Rename collapsible container#1993
Conversation
akrolsmir
left a comment
akrolsmir
left a comment
There was a problem hiding this comment.
"expander" seems fine for the name!
I'm conflicted on keeping the name "collapsible" for internal usage (e.g. throughout our codebase, as the name for the protobuf and also in the withCollapsible text).
In favor: Separation between the public-facing name and our internal name makes it more kosher to change the public-facing name in the future. Maybe there will be other "collapsible" widgets that aren't aren't "expanders"?
Against: It's more friction for some future code reader to have to juggle "what is the difference between an expander and a collapsible"? Simple things like ctrl+f get more annoying, Given that we've already discussed this name to death, it's not super likely that we'll want to change it.
Leaning against; WDYT?
|
I went with in favor mostly because I was thinking of a possible collapsible card in the future. I can change it to expandable so its more consistent? But I think as a HOC and proto, an adjective is more preferable over a noun either way? It's not naming the component really but more so defining an attribute on other components |
akrolsmir
left a comment
akrolsmir
left a comment
There was a problem hiding this comment.
Yeah, as discussed I'd like to match up the external name expander with the internal one expandable (in place of collapsible today). That could be a different PR though!
karriebear
force-pushed
the
rename-collapsible-container
branch
from
c5fc100 to
deaeaee
Compare
karriebear
force-pushed
the
rename-collapsible-container
branch
from
deaeaee to
a12d990
Compare
* [Horizontal Layout] Implement st.container() (#1961) * Fix up dg._block() to be dg.container() * Add a unit test * Add an e2e test for st.container * Minor cleanups * Clean up variable names and comments * Clean up deltaPath code and docs * [Horizontal Layout] Implement `with DG` (#1964) * Force reinstall of setuptools to 49.6.0 (#1962) * try updating in virtualenv * new cache * am i using 39.0.1 and if so why is it failing? * somewhere somehow we are upgrading to setuptools 50.0.3... but where..? * looks like somewhere between setup + pipenv-install we get a newer version of setuptools * looks like somewhere between setup + pipenv-install we get a newer version of setuptools * force reinstall * looks like it's test requirements messing us up? * force reinstall + cleanup debugs * Fix up dg._block() to be dg.container() * Add a unit test * Add an e2e test for st.container * Minor cleanups * Clean up variable names and comments * Clean up deltaPath code and docs * Organize DGs in a tree, to track the `with DG` * Enable `with` in every st.foo call * Appease mypy with # type: ignore * More sacrifices at the altar of mypy * Add unit and e2e tests for with * Actually, don't rely on each DG mixin to call _active_dg Revert "Appease mypy with # type: ignore" This reverts commit 4ba7c47. Revert "Enable `with` in every st.foo call" This reverts commit 9b467d6. * Use the active DG for all "protected" class methods * Clean up DG docs * Don't switch DGs from add_rows Per Karrie's suggestion Co-authored-by: karrie <[email protected]> * [horiz-layout] Add collapsible_container (#1978) * Add collapsible_container * Add default collapsed * make label required + initial docs * Consolidate with WIP column work * Address PR comments for more cleanliness * Update e2e tests * Rename collapsible container (#1993) * rename to expander * Fix tests * Rename collapsible to expandable * [Horiz Layout] Columns (#1979) * Implement columns with variable widths * Add a e2e test for columns * Add a unit test * Update Prettier to 1.19 to allow optional chaining https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Optional_chaining * Fix e2e test * Downgrade Prettier back to 1.18 * Empty columns now take up space * Remove duplicate function from merge conflict * Use flex-gap instead of margin, so column sides don't get padded * Minor tweaks requested by code review * Prefix new layout methods with `beta_` Co-authored-by: karrie <[email protected]>
2 participants
Description: Rename
collapsible_containertoexpanderon the python side. Ideally would like other elements in the future to possibly usewithCollapsibleif appropriate so leaving frontend as isContribution License Agreement
By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license.