appsmith
appsmith copied to clipboard
[Epic] Dynamically Change height for widgets (New Size Options for Container and other widgets)
Objective
Enable widgets to mimic standard HTML block elements by automatically extending the height to fit contents.
Success Metrics *
Goal | Metric |
---|---|
Mimic HTML block component behaviour | Feature enabled numbers in deployed apps? (TBD) |
Requirements
Requirement | User story |
---|---|
Block or "Container" like widgets extend height to fit all contents | Web developers are used to seeing block elements extend until configured with a fixed height (Standard HTML block element behaviour). Appsmith developers can now find a parallel behaviour in Appsmith |
Elements have a concept of max-height, a configurable property | One of the common CSS properties used by web developers to restrict how much an element can "grow" in size, is using the max-height property. Appsmith developers can now mimic this in Appsmith |
Hide this feature from certain widgets, that do not allow this behaviour | Not all HTML elements increase in height based on contents, especially the ones which don't have contents like br and hr , similarly, widget developers can configure if a widget can extend height dynamically in widget configurations |
DX updates
-
Widgets will have a new property called Height with two options in a dropdown
FIXED
,HUG_CONTENTS
-
When the property above is
HUG_CONTENTS
, Appsmith developers will see more properties calledMax Height in Rows
:number
andMin Height in Rows
:number
. These define the maximum/minimum height to which the widget can grow/shrink automatically. -
Default Min Height is 2 rows (this is also the minimum value which can be configured), while Default Max Height is 1000 rows or some arbitrarily large value.
-
FIXED
: This is the current mode for widgets, where the heights once defined, shall not be changed -
HUG_CONTENTS
:- In the edit mode, the height will change dynamically, and Appsmith developers will not be allowed to manually resize the height. These changes will be persisted in the DSL.
- In the view mode, the height will change dynamically but the values will not be persisted in the DSL.
Note: These properties do not conflict with "Scroll contents" property as once the height is dynamic, the widget will not have a scroll
Documentation
- A detailed documentation with screen recordings demonstrating the logic of reflow
- A detailed documentation with screen recordings demonstrating the two new properties
Reflow Logic (UX)
- ~~Reflow logic will follow the current drag and drop reflow logic for consistency.~~ This has been changed to have relative reflow (See comment)
- ~~If Appsmith developers need to maintain relative positions of widgets, the widgets can be grouped into a transparent container.~~
Example demonstration of before and after:
- New Images TBD
Developer Handoff Document in Figma
https://www.figma.com/file/Yk4V1PQFbL9Cmti1lemqFd/?node-id=442%3A90845
RACI Matrix
Role | Members |
---|---|
Responsible | @riodeuno @ankurrsinghal |
Accountable | @riodeuno |
Consulted | @momcilo-appsmith @areyabhishek @Nikhil-Nandagopal @rahulramesha @vinay-appsmith |
Informed | @somangshu @vuiets @rohitagarwal88 |
@momcilo-appsmith i understood the concept, but did not understand how this can be built. Keeping dynamic height aside for a min.
Main area of confusion was how will we align 2 items to be in the same line/row. Here it automatically splits the container into multiple rows and only adds a new row in hug contents. Does it mean we need to start thinking in terms of rows in containers?
@dilippitchika We don't need to align them, that is just how it had to be shown in Figma.
From #4024 we also wanted to allow dynamic helper text that is below the input component and needs additional height, It will have to reflow other widgets. Our alternate solution before this was to introduce a tooltip for helper.
Another conversation on the same topic in a private thread.
@areyabhishek : We seriously need to fix our text widget. The weird vertical way we cut the top and bottom of the widget when there's no space and the way it introduces the scroll is baad. Every app looks terrible on my screen in edit mode because the text is always chopped off.
@momcilo-appsmith thank you for writing this it looks great! I have a few points here
- Scroll and this property do seem to be related and if a user enables dynamic height, then scroll should be hidden
- Instead of having number of rows as a max height which a user has to guess, why not simply have a property "Enable Max Height" which sets the current height as the max height? This is easier for a user to configure what the correct max height should be.
@Nikhil-Nandagopal
-
We can extend height to a limit, after which the developer needs to decide whether the content is clipped (today's default), or content is scrolled. This is why I believe these are exclusive. How do you imagine this feature working in conjunction with scroll?
-
Regarding the max height -- do you mean, something like "Use current height as max height"?
@riodeuno I think my point is scroll will only come if there is a max height set otherwise it won't Yes just like that "Use current height as max height". This makes it easier to configure than guessing what the UI might look like and what the correct max height value should be
I think my point is scroll will only come if there is a max height set otherwise it won't
Agreed!
This makes it easier to configure than guessing what the UI might look like and what the correct max height value should be
Can you elaborate the process? I am trying to imagine the DX as follows
- Developer enables dynamic height
- Developer resizes widget to increase height
- Developer enables "use current height as max height"
- Developer resizes widget back to decrease height
One alternative I can think of, regarding the numerical row input is to have a marker which marks a widget which has a dynamic height.
@riodeuno got the problem thanks for clarifying. My suggestion DX doesn't make sense then :) Can we think of something better for this? Guessing the correct max width / height has always been one of my struggles as a developer. The UI always goes bonkers when I can't visualize what the worst case can look like
@Nikhil-Nandagopal Sounds good. I can relate to the inaccuracy in anticipating how a feature effects things at runtime. 😅
@momcilo-appsmith Any suggestions on a good UX here?
Adding screenshots to demonstrate the experience we want to create
Update
-
Why are we not using dynamic width?
- Appsmith doesn't have a concept of infinitely wide canvas. There is always a fixed maximum number of columns a widget can occupy.
- In web design, horizontal scrolls are usually not a good idea, as mouse users cannot easily navigate this without buttons.
- If we really do need dynamic width, we'll look into it as a subsequent effort.
-
Why max-height?
- This constraint is what web developers have come to expect when something changes dimensions in an uncontrolled manner.
- It is better to have this option, as a feasibility improvement, than not having it.
-
Will this work in edit mode for container like widgets?
- Yes. Which means that if we drag or resize a child of the container widget, the container widget should increase height to adapt to this, if needed.
-
Appsmith developers can be surprised with the changes in layout in the view mode!!
-
Causes for surprises (identified so far):
- No idea what 100 rows of max-height means.
- No idea what widgets might get displaced when a widget extends height
-
Solutions:
- UX to indicate possible changes to a widget's height, without making the UI too busy
- Relative reflow. Maintains padding with the "pushed" widgets, and within extending containers, as that's how we expect it to work in a web application
-
-
How will this handle the invisible widgets?
- It will consider them as existing widgets in edit mode
- It will consider them non-existent widgets in view mode.
- Special handling is out of scope for this project
- We will try to accommodate discoverability when dealing with the UX mentioned in the previous point.
cc: @Nikhil-Nandagopal @momcilo-appsmith @areyabhishek
@dilippitchika @riodeuno we also need to define which widgets will be dynamic by default vs which will need a user opt-in. Because for example if table is dynamic by default, it will crash the app. Similarly, dynamic height in the chart, audio widget, iframe doesn't make sense. We need to think carefully how this will affect every single widget
@Nikhil-Nandagopal Absolutely. I'll make sure to add this to the task list for the project.
Action Items
Upcoming week
- [ ] Feature Implementation design @riodeuno @ankurrsinghal
- [ ] Review of code design with emphasis on performance with @SatishGandham, @mohanarpit and @rahulramesha
- [x] Identify all widgets that needs this (Parallel task) @somangshu @dilippitchika @riodeuno
Spinoff project ( This will run in parallel to implementation related tasks )
- [x] Design solution for
Appsmith developers can be surprised with the changes in layout in the view mode!!
@momcilo-appsmith
cc @rohitagarwal88
@riodeuno @ankurrsinghal Here is the Handoff file: https://www.figma.com/file/VtsF6ygzHI6gmsOZC3rGnK/Dynamic-Height-Mon-14-March-2022?node-id=2%3A1 Please take a look and let me know if you have any questions or feedback.
There's something which I wanted to raise for table and list widget, we have a pagesize which is calculated for each table and list. How will the max height property be used if the data is coming according to the page size which is currently set?
The only scenario i found this useful was to prevent scroll on word-wrap. @somangshu @riodeuno
CC - @sbalaji1192
@dilippitchika This is a great point! I believe, if we can confirm that the dynamic height will come into play only if the word-wrap is enabled, then we can make sure to make the dynamic height property available accordingly.
Dynamic height investigation results
Approach
- We assumed using
ref
to be the most straightforward approach to enable dynamic height in widgets. - This works by attaching refs to all widget components, that are forwarded from the Widget in concern. The refs are creatd in the BaseWidget abstract to make sure they're expected to be avaialable in all widgets whether they decide to enable dynamic height or not.
- We identified the widgets within our current set of widgets -- that would have an impact to Appsmith application development -- that will have the dynamic height option.
- We looked into how to forward refs within the list of widgets identified.
- We use the
scrollHeight
property of the component's wrapping element to determine if the widget's height needs to be adjusted. This means, that all components should cover all of the widget's area.
General findings
-
All widgets will have a new prop passed down to their respective components --
ref
-- which is defined in the BaseWidget (React.createRef
), hence can be accessed usingthis.contentRef
in any of our widgets. -
All widget components will now need to have to be wrapped into the
React.forwardRef
HOC to safely propagate these refs. -
Some widget components utilise refs internally, if these refs are added to the top level wrapping element in the component, then they will need to re-use the
ref
passed down from the widget, instead of creating a ref within the component. -
All widget components need to be wrapped in an HTMLElement (via
styled-components
or otherwise). This means that usage of custom components fromblueprintjs
or other libraries which donot forward the ref prop safely to their underlying HTML element is forbidden. For example, we can't use theText
component provided byblueprintjs
as the top level wrapper in theTextWidget
anymore. * Note: This is still under investigation, if we can find a way to manage this, this requirement may be removed * -
Based on the above observations, we need to be also making changes to the widget components in case they violate any of the rules above.
-
We will be propagating the dynamic heights via a redux state instead of computing it on the fly in the Base Widget for the following reasons
- Changes in one widget may cause changes in the ancestors as well
- Changes in one widget need to reflow other widgets based on the empty space created or new space occupied
- To avoid re-renders and make sure computations only update state when necessary.
Collateral fixes
- Empty space at the bottom in the Main Container in view mode can be removed with this solution
- Empty space created dynamically (hiding widgets, or reducing their height) can potentially be filled by reflowing other widgets
- A more detailed template for widget components will be created. Not really a fix.
Concerns
- Standardisation: We need a way to enforce the standards for component code, so that widget developers cannot fail to have the dynamic height work correctly for the widgets they implement.
- Filling empty space created when a widget is hidden, is still under product discussions and will need more time to reach a conclusion, leading to a potential code change in later stages of the project.
Canvas Widget
-
This widget has a few quirks where it differs in how it renders on the edit mode and the view mode. We've looked into it to a certain extent, not all quirks have been tackled, however, we are confident that this will not be a blocker for the project.
-
A few points of note
- CanvasWidget has more rows than its parent if it
canExtend
, i.e, if the parent can scroll contents. - CanvasWidget renders the
DropTarget
in the edit mode and renders a simplediv
in view mode. - CanvasWidget already has dynamic height, in all scenarios, it is the parent which needs to adapt to the changes in the height of the Canvas Widget based on the Canvas Widget's contents. This is another quirk, because an update to the parent's height, causes an update in the CanvasWidget's height, which if the dynamic height feature doesn't handle, will cause the parent's height to increase again, leading to an infinite loop.
- CanvasWidget has more rows than its parent if it
Container Widget
- Already has a block
div
element as its child, which is the Canvas Widget. Changes in the Canvas Widget's contents will cause the Container Widget to change in height. - Refs are propagated Canvas Widget whose
scrollHeight
is the property to decided any changes. - Refs were being used in the container component which now need to move to the context ref
Form Widget
- Same as the Container Widget
Tabs Widget
- Same as the Container Widget
List Widget
- Needs product update, we haven't investigated this yet.
Switch Group Widget
- Using the
React.forwardRef
, we can set the base widgetcontentRef
to the SwitchGroupContainer(which is a normal styled div) ref and as soon as we have more options the base widget will grow dynamically.
Checkbox Group Widget
- Same as Switch Group Widget
Radio Group Widget
- Same as Switch Group Widget but here is one issue, the Radio group container is a styled
RadioGroup
component from blueprint so we can't access the internal ref to the div. To fix this we can wrap the container in another simple plain div and forward the ref to its ref.
Rating Widget
- Same as Switch Group Widget but here the issue is it does not updates on mount but on update.
Table Widget
- Table widget needs Dynamic Height when we enable Server Side Pagination, where we forward the ref to
table-body
container. - One issue here is that, reflow wrapper is setting some computed height which inhibits the table container to grow as much as the base widget height which updates because of ref's
scrollHeight
.
Input Widget
- Input widget problem arises when we have a very long label it overflows the input field to fit the container.
- One thing we can do using Dynamic Height is that as soon as base widget updates its height using
scrollHeight
, the label and input field becomes block level which looks better.
Checkbox Widget
- Same as input widget.
Switch Widget
- Same as input widget.
Input Currency Widget
- Same as input widget.
Input Phone Widget
- Same as input widget.
Select Widget
- Same as input widget.
Multi-select Widget
- Same as input widget.
Tree select Widget
- Same as input widget.
Multi-select Tree Widget
- Same as input widget.
Datepicker Widget
- Same as input widget (Currently, Datepicker does not have a label property).
JSONForm Widget
- Same as Switch Group Widget. Here we will forward the ref to
StyledFormBody
's ref.
Update
- Conclusion: Disable vertical resize of the widget when dynamic height is enabled
- Parallel design exploration: Signifiers of max-height and min-height. @vinay-appsmith @momcilo-appsmith
- Conclusion: Persist heights in edit mode when they change dynamically.
cc @rohitagarwal88 @ankurrsinghal
Update 6th May:
- Min-Max height signifiers Design - In progress @momcilo-appsmith @vinay-appsmith
- POC development - In Progress @ankurrsinghal @riodeuno - ETA 16th May
- Technical hurdle: Undo-Redo does not work well with the dynamic height - Details to follow.
Update 27th May
Tasks for this week:
- [ ] @riodeuno Complete the dynamic height layout update algorithm
- [ ] @ankurrsinghal Complete implementing updates to the widgets to have them respond correctly to content changes
- [ ] @btsgh @Sripriya93 Work on compiling test scenarios and strategy for QA of the feature.
- [ ] @rohitagarwal88 Facilitate communication with the App viewers for resolving blockers on @ankurrsinghal 's tasks
For today and the upcoming week these are my actionable:-
-
Resolving the conflicts of the base widget branch to fix the flaky tests in 2 pending PR's for standardization.
-
Fixing CSS of Label Widgets:-
- Input, InputPhone, InputCurrency
- Multi-Select, Multi-Tree Select, Select, Tree Select
- Switch Group, Radio Group, Checkbox Group
-
Fixing the CSS of Label of Switch, Checkbox widgets.
-
Fixing the CSS of JSON Form Widget (if any).
-
Fixing the CSS/JS of Table widget where it scrolls 2 rows less.
-
Fixing the CSS of Text widget height issue.
-
Beginning the design of th overlay for min and max height if I have bandwidth left.
@ankurrsinghal @riodeuno is there a DP we can try out?
@Nikhil-Nandagopal Not yet. We're completing some of the core logic related to the height updates. As well as the actionable posted by @ankurrsinghal.We should be able to share deploy preview to try this out early next week.
Update 13th June
- [x] General design frozen for the min-max signifiers @momcilo-appsmith
- [x] To understand if percentage is a better unit than rows, we'll create a PoC for the percentages. It is confirmed that w.r.t designs there will not be any major changes in either case. @momcilo-appsmith @riodeuno
- [x] Container update logic for widget addition, move, deletion, cut, paste, undo and redo. (This has bugs which we'll resolve before sharing the deploy preview for feedback) @rahulramesha
Targets for this week.
- [ ] Share POC with the team, without the DX designed by @momcilo-appsmith @riodeuno @ankurrsinghal
- [x] Remove Dynamic height project from this week's sprint backlog to avoid skewing numbers on zenhub @rohitagarwal88
- [x] Split the DX implementation task into granular issues and associate it with the epic @riodeuno
- [x] Overall Test strategy, QA alignment to goals and detailed checklist for test cases. @btsgh @Sripriya93
@Nikhil-Nandagopal Here is an early preview of the feature. This doesn't have the visual signifiers, but has all features implemented. We're dealing with some issues, hence it is an early preview.
https://appsmith-kqx3h5521-get-appsmith.vercel.app/
@Nikhil-Nandagopal @btsgh @Sripriya93 A new POC with range sliders for min max controls is available for feedback. Please check it out: https://appsmith-git-poc-dynamic-height-range-slider-get-appsmith.vercel.app/
@Nikhil-Nandagopal Thank you for your reply #16783. As you said, we can consider making this a property of the table. I will add a property to TableWidget2 to let users choose whether to use fixed pageSize or dynamically calculate pageSize based on dragging or resizing.
Hello. I was encouraged by Appsmith support (Amelia) to write a comment here, as I requested being able to make a list view with dynamic height (so all list items is visible on the page, without pagination).
I don't remember when I've last made a fixed sized window in a web app, and expected pagination to be an option rather than a requirement. It also turned hard to decide how tall to make the listview, as some phones would then show the pagination, but others would have the pagination bar outside the bottom of the screen, and thus people had to scroll anyways to reach it. Another solution was to make it small enough to fit all mobile screens, but then we have the next issue with big phones having lots of wasted space.
I see the ListView (and other views no having dynamic height) the same as if this GitHub issue would have a fixed height box, containing all the comments, which you could scroll inside. I don't think that makes sense, compared to what they actually have, where you scroll down through the page, and in the bottom you'll find the commenting section :)
I hope this feature will be part of Appsmith, as I otherwise like what Appsmith seems to offer 👍