-
Notifications
You must be signed in to change notification settings - Fork 3.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: Refactor WDSTableWidget with basic features #33286
Changes from 10 commits
78e794a
9c80a63
f9d85fd
59d241b
aa9870b
39abea1
280baec
f18fd72
124ee74
4a11198
2b93f29
63c6c25
c70107e
c0dc998
f8fb895
618ef03
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
export * from "./src"; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
import React from "react"; | ||
|
||
import { Text } from "../../Text"; | ||
import type { LinkProps } from "./types"; | ||
import styles from "./styles.module.css"; | ||
|
||
export function Link(props: LinkProps) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We have to use Link from |
||
const { children, href, rel, target, ...rest } = props; | ||
|
||
return ( | ||
<Text {...rest}> | ||
<a className={styles.link} href={href} rel={rel} target={target}> | ||
{children} | ||
</a> | ||
</Text> | ||
); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
export * from "./Link"; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
.link { | ||
text-decoration: underline; | ||
text-decoration-color: currentColor; | ||
text-underline-offset: 2px; | ||
|
||
&:hover { | ||
color: currentColor; | ||
text-decoration-color: currentColor; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
import type { ComponentProps } from "react"; | ||
|
||
import type { TextProps } from "../../Text"; | ||
|
||
export interface LinkProps extends TextProps { | ||
href?: string; | ||
target?: ComponentProps<"a">["target"]; | ||
rel?: string; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1,2 @@ | ||
export { Text } from "./Text"; | ||
export type { TextProps } from "./types"; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -29,7 +29,7 @@ export interface TextProps { | |
/** Sets the horizontal alignment of the inline-level content inside a block element or table-cell box. See [MDN](https://developer.mozilla.org/en-US/docs/Web/CSS/text-align). | ||
* @default left | ||
*/ | ||
textAlign?: "left" | "center" | "right"; | ||
textAlign?: "start" | "center" | "end" | "left" | "right"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we still need left and right here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we can remove it. Let me remove them. |
||
/** Allows limiting of the contents of a block to the specified number of lines. See [MDN](https://developer.mozilla.org/en-US/docs/Web/CSS/-webkit-line-clamp). */ | ||
lineClamp?: number; | ||
/** Sets the CSS [className](https://developer.mozilla.org/en-US/docs/Web/API/Element/className) for the element. Only use as a **last resort**. Use style props instead. */ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -201,22 +201,24 @@ class PrimaryColumnsControlV2 extends BaseControl<ControlProps, State> { | |
const isFocused = | ||
!_.isNull(this.state.focusedIndex) && | ||
_.includes(this.state.duplicateColumnIds, column?.id); | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The pattern for this should be that TableWidget module exports the control, which is then registered by Appsmith. This way, this control can be as specific as necessary to the TableWidget. @jsartisan What do you think? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I agree. We have to make a system where widgets can also register custom controls in the control registry. |
||
return ( | ||
<> | ||
<div className="flex pt-2 pb-2 justify-between"> | ||
<div>{Object.values(reorderedColumns).length} columns</div> | ||
{this.isEditableColumnPresent() && ( | ||
<EdtiableCheckboxWrapper | ||
className="flex t--uber-editable-checkbox" | ||
rightPadding={this.state.hasScrollableList} | ||
> | ||
<span className="mr-2">Editable</span> | ||
<Checkbox | ||
isSelected={this.isAllColumnsEditable()} | ||
onChange={this.toggleAllColumnsEditability} | ||
/> | ||
</EdtiableCheckboxWrapper> | ||
)} | ||
{this.props.widgetProperties.type !== "WDS_TABLE_WIDGET" && | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hiding the editable checkbox for WDSTableWidget There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is this necessary? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we don't want to show the editable icon for wds table widget. This control is being used in TableWidgetV2 as well. So had to add a check. Another way is to create the whole control and use it in table widget. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This is the preferred approach, and we have to promote a complete separation from the legacy. |
||
this.isEditableColumnPresent() && ( | ||
<EdtiableCheckboxWrapper | ||
className="flex t--uber-editable-checkbox" | ||
rightPadding={this.state.hasScrollableList} | ||
> | ||
<span className="mr-2">Editable</span> | ||
<Checkbox | ||
isSelected={this.isAllColumnsEditable()} | ||
onChange={this.toggleAllColumnsEditability} | ||
/> | ||
</EdtiableCheckboxWrapper> | ||
)} | ||
</div> | ||
<div className="flex flex-col w-full gap-1"> | ||
<EvaluatedValuePopupWrapper {...this.props} isFocused={isFocused}> | ||
|
@@ -233,26 +235,34 @@ class PrimaryColumnsControlV2 extends BaseControl<ControlProps, State> { | |
renderComponent={(props: any) => | ||
DraggableListCard({ | ||
...props, | ||
showCheckbox: true, | ||
showCheckbox: | ||
this.props.widgetProperties.type !== "WDS_TABLE_WIDGET" && | ||
true, | ||
placeholder: "Column title", | ||
}) | ||
} | ||
toggleCheckbox={this.toggleCheckbox} | ||
toggleVisibility={this.toggleVisibility} | ||
toggleVisibility={ | ||
this.props.widgetProperties.type !== "WDS_TABLE_WIDGET" | ||
? this.toggleVisibility | ||
: undefined | ||
} | ||
updateFocus={this.updateFocus} | ||
updateItems={this.updateItems} | ||
updateOption={this.updateOption} | ||
/> | ||
</EvaluatedValuePopupWrapper> | ||
<Button | ||
className="self-end t--add-column-btn" | ||
kind="tertiary" | ||
onClick={this.addNewColumn} | ||
size="sm" | ||
startIcon="plus" | ||
> | ||
Add new column | ||
</Button> | ||
{this.props.widgetProperties.type !== "WDS_TABLE_WIDGET" && ( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hiding the Add the new column button for WDSTableWidget |
||
<Button | ||
className="self-end t--add-column-btn" | ||
kind="tertiary" | ||
onClick={this.addNewColumn} | ||
size="sm" | ||
startIcon="plus" | ||
> | ||
Add new column | ||
</Button> | ||
)} | ||
</div> | ||
</> | ||
); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this necessary?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was a typo in the LightModeTheme.
bgNegativeSubtle
was using the function ofbgNegativeActive