docs: Add comprehensive README for ButtonWidget#41658
Open
xingzihai wants to merge 3 commits intoappsmithorg:releasefrom
Open
docs: Add comprehensive README for ButtonWidget#41658xingzihai wants to merge 3 commits intoappsmithorg:releasefrom
xingzihai wants to merge 3 commits intoappsmithorg:releasefrom
Conversation
This commit adds accessibility improvements to the Button and Select components in the Appsmith Design System (ads). Button component improvements: - Add aria-busy attribute for loading state (WCAG 4.1.2) - Ensure aria-label is properly passed through for icon-only buttons Select component improvements: - Add isRequired prop with aria-required attribute (WCAG 4.1.2) - Add aria-invalid attribute for validation states (WCAG 4.1.2) - Add aria-busy attribute for loading state - Support aria-label, aria-labelledby, and aria-describedby props These changes improve the accessibility of the design system components for screen reader users and follow WCAG 2.1 guidelines.
Add comprehensive accessibility tests for: - RadioButtonGroup: ARIA attributes, keyboard navigation, error states - Collapsible: expand/collapse accessibility, keyboard support - EditableText: edit interactions, validation, focus management - Button: disabled/loading states, keyboard accessibility, focus management These tests ensure compliance with WCAG guidelines and improve accessibility for users relying on assistive technologies.
- Add detailed documentation for Button widget - Include property reference tables - Add usage examples and best practices - Document events and methods - Provide common use cases with code samples This documentation aims to help developers better understand and utilize the Button widget in their Appsmith applications.
Contributor
WalkthroughThis PR enhances accessibility across the design system by introducing ARIA attributes (aria-label, aria-busy, aria-invalid, aria-required) to the Button and Select components, adds an Changes
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (3)
app/client/src/components/editorComponents/Button.test.tsx (2)
341-347: Icon-only accessibility test should assert an accessible name.Right now this only checks render success. Please assert a name source (
aria-labelor equivalent), otherwise this test won’t catch icon-only a11y regressions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/client/src/components/editorComponents/Button.test.tsx` around lines 341 - 347, In the "should be accessible when only icon is provided" test, after calling renderButton({ icon: "plus", text: undefined }) assert that the button has an accessible name: use the rendered element from screen.getByRole("button") and either expect(button).toHaveAttribute("aria-label", "<expected label>") or expect(button).toHaveAccessibleName("<expected label>") (or use screen.getByRole("button", { name: /plus/i }) if the label is known); update the test to fail when no accessible name is present so icon-only a11y regressions are caught (referencing renderButton and the test title).
296-321: Keyboard activation tests are non-assertive.Both tests state activation behavior but only assert DOM presence. Add callback assertions (or rename tests) so they actually guard keyboard interaction behavior.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/client/src/components/editorComponents/Button.test.tsx` around lines 296 - 321, Update the two keyboard tests ("should trigger click on Enter key" and "should trigger click on Space key") to assert that the provided onClick callback is invoked: keep using renderButton({ onClick }) and screen.getByRole("button"), fire the appropriate key events (for Enter: keyDown and keyUp with key "Enter"; for Space: keyDown and keyUp with key " " / code "Space"), then add assertions like expect(onClick).toHaveBeenCalled() or toHaveBeenCalledTimes(1) to verify that the native button activation actually triggered the onClick handler.app/client/src/components/common/Collapsible.test.tsx (1)
75-97: Keyboard test block is currently mouse-only.This section only verifies click paths; consider adding Enter/Space/Tab assertions (or rename the block to interaction behavior) so keyboard regressions are actually covered.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/client/src/components/common/Collapsible.test.tsx` around lines 75 - 97, The "Keyboard Accessibility" tests only simulate mouse clicks; update the tests around renderCollapsible and handleCustomCollapse to also assert keyboard activation by focusing the label (closest(".icon-text") from screen.getByText("Test Collapsible")) and simulating Enter and Space key events (e.g., fireEvent.keyDown or userEvent.keyboard) and/or tab focus to ensure handleCustomCollapse is called; alternatively, rename the describe block to "Click and Keyboard Interaction" and add separate it cases that simulate Tab+Enter and Tab+Space to validate keyboard activation of the collapse control.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/client/src/components/editorComponents/Button.test.tsx`:
- Around line 237-247: The test titled "should not call onClick when href is
provided (navigation behavior)" currently only checks the presence of the link;
update the assertion to verify the click handler was not invoked by adding an
assertion such as expect(onClick).not.toHaveBeenCalled() (or
expect(onClick).toHaveBeenCalledTimes(0)) after fireEvent.click(link). Keep the
existing setup using renderButton and the link lookup but add the onClick
call-count assertion to validate the test claim.
In `@app/client/src/components/editorComponents/EditableText.test.tsx`:
- Around line 122-130: The waitFor callback must not perform side-effectful
interactions; instead, use waitFor to assert the input exists (e.g., waitFor(()
=> expect(document.querySelector(".bp3-editable-text-input")).toBeTruthy())) and
then do the fireEvent.change and fireEvent.keyDown on the selected element
outside of waitFor. Update the occurrences that use
document.querySelector(".bp3-editable-text-input") in EditableText.test.tsx (the
blocks around the current fireEvent calls) to first waitFor/assert the element
is present, then perform the change/Enter key events after that assertion; apply
this same pattern to the other listed occurrences so no fireEvent calls are
executed inside waitFor.
In `@app/client/src/widgets/ButtonWidget/README.md`:
- Around line 35-37: The README for the Button widget uses two different
property names for the button caption (Label vs text); standardize on a single
canonical property (choose either Label or text) throughout the document and
examples to avoid confusion: update every instance of "Label" and all examples
that reference `text` so they use the chosen property consistently (search for
"Label", "`text`", "Button widget", and the examples around lines referenced in
the review) and ensure any code snippets, docs, and example usages use the same
property name and description.
- Around line 41-45: Two fenced code blocks in the ButtonWidget README examples
are missing language identifiers, triggering markdownlint MD040; update the code
fences in README.md for the ButtonWidget examples to include a language tag
(e.g., use ```text or ```javascript) so both the configuration snippet
(Label/onClick example) and the button label guidance block declare their
language; locate the fenced blocks in the ButtonWidget README and prefix the
opening ``` with the chosen language identifier for each block.
- Line 54: Update the `onClick` prop description in ButtonWidget README so it is
a single complete sentence: replace the fragment "Action to execute when the
button is clicked. Can be an API call, query, or JavaScript function" with a
single sentence such as "Action to execute when the button is clicked; it can be
an API call, query, or JavaScript function." Ensure you edit the `onClick` row
in the ButtonWidget README (`onClick` prop description) to reflect this wording.
- Around line 363-368: The example references a non-existent generic widget name
TableRow; replace those references with the actual table widget used in other
examples (e.g., Table1) and use its concrete selection properties (e.g.,
Table1.isSelected or Table1.selectedRow and Table1.selectedRow.id) so the
bindings become executable: update the expressions in text, isDisabled,
buttonVariant and onClick to reference the concrete widget name and its
selectedRow.id and selection flag instead of TableRow.
- Line 396: Update the relative link in the ButtonWidget README: change the
CONTRIBUTING.md path from ../../../../CONTRIBUTING.md to
../../../../../CONTRIBUTING.md so the link resolves to the repository root;
locate the markdown line that reads "For more information about contributing to
Appsmith, see the [Contributing Guide](../../../../CONTRIBUTING.md)." and
replace the path accordingly.
---
Nitpick comments:
In `@app/client/src/components/common/Collapsible.test.tsx`:
- Around line 75-97: The "Keyboard Accessibility" tests only simulate mouse
clicks; update the tests around renderCollapsible and handleCustomCollapse to
also assert keyboard activation by focusing the label (closest(".icon-text")
from screen.getByText("Test Collapsible")) and simulating Enter and Space key
events (e.g., fireEvent.keyDown or userEvent.keyboard) and/or tab focus to
ensure handleCustomCollapse is called; alternatively, rename the describe block
to "Click and Keyboard Interaction" and add separate it cases that simulate
Tab+Enter and Tab+Space to validate keyboard activation of the collapse control.
In `@app/client/src/components/editorComponents/Button.test.tsx`:
- Around line 341-347: In the "should be accessible when only icon is provided"
test, after calling renderButton({ icon: "plus", text: undefined }) assert that
the button has an accessible name: use the rendered element from
screen.getByRole("button") and either
expect(button).toHaveAttribute("aria-label", "<expected label>") or
expect(button).toHaveAccessibleName("<expected label>") (or use
screen.getByRole("button", { name: /plus/i }) if the label is known); update the
test to fail when no accessible name is present so icon-only a11y regressions
are caught (referencing renderButton and the test title).
- Around line 296-321: Update the two keyboard tests ("should trigger click on
Enter key" and "should trigger click on Space key") to assert that the provided
onClick callback is invoked: keep using renderButton({ onClick }) and
screen.getByRole("button"), fire the appropriate key events (for Enter: keyDown
and keyUp with key "Enter"; for Space: keyDown and keyUp with key " " / code
"Space"), then add assertions like expect(onClick).toHaveBeenCalled() or
toHaveBeenCalledTimes(1) to verify that the native button activation actually
triggered the onClick handler.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5db0b091-d6af-4482-96da-6a27843291df
📒 Files selected for processing (8)
app/client/packages/design-system/ads/src/Button/Button.tsxapp/client/packages/design-system/ads/src/Select/Select.tsxapp/client/packages/design-system/ads/src/Select/Select.types.tsapp/client/src/components/common/Collapsible.test.tsxapp/client/src/components/editorComponents/Button.test.tsxapp/client/src/components/editorComponents/EditableText.test.tsxapp/client/src/components/editorComponents/RadioButtonGroup.test.tsxapp/client/src/widgets/ButtonWidget/README.md
Comment on lines
+237
to
+247
| it("should not call onClick when href is provided (navigation behavior)", () => { | ||
| const onClick = jest.fn(); | ||
| renderButton({ href: "https://example.com", onClick }); | ||
|
|
||
| const link = screen.getByRole("link"); | ||
| fireEvent.click(link); | ||
|
|
||
| // Anchor buttons typically don't call onClick in the same way | ||
| // The navigation is handled by the href attribute | ||
| expect(link).toBeInTheDocument(); | ||
| }); |
Contributor
There was a problem hiding this comment.
This test doesn’t verify its claim yet.
The title says onClick should not be called for href, but the assertion only checks element presence. Please assert onClick call count directly.
Suggested fix
it("should not call onClick when href is provided (navigation behavior)", () => {
const onClick = jest.fn();
renderButton({ href: "https://example.com", onClick });
const link = screen.getByRole("link");
fireEvent.click(link);
- // Anchor buttons typically don't call onClick in the same way
- // The navigation is handled by the href attribute
- expect(link).toBeInTheDocument();
+ expect(onClick).not.toHaveBeenCalled();
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| it("should not call onClick when href is provided (navigation behavior)", () => { | |
| const onClick = jest.fn(); | |
| renderButton({ href: "https://example.com", onClick }); | |
| const link = screen.getByRole("link"); | |
| fireEvent.click(link); | |
| // Anchor buttons typically don't call onClick in the same way | |
| // The navigation is handled by the href attribute | |
| expect(link).toBeInTheDocument(); | |
| }); | |
| it("should not call onClick when href is provided (navigation behavior)", () => { | |
| const onClick = jest.fn(); | |
| renderButton({ href: "https://example.com", onClick }); | |
| const link = screen.getByRole("link"); | |
| fireEvent.click(link); | |
| expect(onClick).not.toHaveBeenCalled(); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/client/src/components/editorComponents/Button.test.tsx` around lines 237
- 247, The test titled "should not call onClick when href is provided
(navigation behavior)" currently only checks the presence of the link; update
the assertion to verify the click handler was not invoked by adding an assertion
such as expect(onClick).not.toHaveBeenCalled() (or
expect(onClick).toHaveBeenCalledTimes(0)) after fireEvent.click(link). Keep the
existing setup using renderButton and the link lookup but add the onClick
call-count assertion to validate the test claim.
Comment on lines
+122
to
+130
| await waitFor(() => { | ||
| const input = document.querySelector( | ||
| ".bp3-editable-text-input", | ||
| ) as HTMLInputElement; | ||
| if (input) { | ||
| fireEvent.change(input, { target: { value: "New Value" } }); | ||
| fireEvent.keyDown(input, { key: "Enter", code: "Enter" }); | ||
| } | ||
| }); |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python - <<'PY'
from pathlib import Path
import re
path = Path("app/client/src/components/editorComponents/EditableText.test.tsx")
text = path.read_text()
pattern = re.compile(r'await\s+waitFor\(\(\)\s*=>\s*{([\s\S]*?)}\);')
for m in pattern.finditer(text):
body = m.group(1)
if "fireEvent." in body and ("if (input)" in body or "if (textarea)" in body):
start = text.count("\n", 0, m.start()) + 1
end = text.count("\n", 0, m.end()) + 1
print(f"Potential flaky waitFor block at lines {start}-{end}")
PYRepository: appsmithorg/appsmith
Length of output: 345
🏁 Script executed:
cat -n app/client/src/components/editorComponents/EditableText.test.tsx | sed -n '120,135p'Repository: appsmithorg/appsmith
Length of output: 620
🏁 Script executed:
cat -n app/client/src/components/editorComponents/EditableText.test.tsx | sed -n '148,160p'Repository: appsmithorg/appsmith
Length of output: 489
🏁 Script executed:
cat -n app/client/src/components/editorComponents/EditableText.test.tsx | sed -n '176,190p'Repository: appsmithorg/appsmith
Length of output: 603
🏁 Script executed:
cat -n app/client/src/components/editorComponents/EditableText.test.tsx | sed -n '209,225p'Repository: appsmithorg/appsmith
Length of output: 637
🏁 Script executed:
cat -n app/client/src/components/editorComponents/EditableText.test.tsx | sed -n '298,310p'Repository: appsmithorg/appsmith
Length of output: 481
🏁 Script executed:
cat -n app/client/src/components/editorComponents/EditableText.test.tsx | sed -n '369,380p'Repository: appsmithorg/appsmith
Length of output: 478
Remove side effects from waitFor callbacks to prevent false-positive tests.
The pattern of executing fireEvent calls inside waitFor with conditional guards creates a flaw: if the input element is missing, the callback silently returns without throwing or executing the interaction, allowing tests to pass incorrectly.
Move assertions into waitFor to validate preconditions, then execute interactions outside:
Corrected pattern
- await waitFor(() => {
- const input = document.querySelector(".bp3-editable-text-input") as HTMLInputElement;
- if (input) {
- fireEvent.change(input, { target: { value: "New Value" } });
- fireEvent.keyDown(input, { key: "Enter", code: "Enter" });
- }
- });
+ const input = await waitFor(() => {
+ const el = document.querySelector(".bp3-editable-text-input") as HTMLInputElement | null;
+ expect(el).toBeInTheDocument();
+ return el as HTMLInputElement;
+ });
+ fireEvent.change(input, { target: { value: "New Value" } });
+ fireEvent.keyDown(input, { key: "Enter", code: "Enter" });Applies to lines 122–130, 150–158, 178–186, 211–219, 300–308, 371–377.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| await waitFor(() => { | |
| const input = document.querySelector( | |
| ".bp3-editable-text-input", | |
| ) as HTMLInputElement; | |
| if (input) { | |
| fireEvent.change(input, { target: { value: "New Value" } }); | |
| fireEvent.keyDown(input, { key: "Enter", code: "Enter" }); | |
| } | |
| }); | |
| const input = await waitFor(() => { | |
| const el = document.querySelector( | |
| ".bp3-editable-text-input", | |
| ) as HTMLInputElement | null; | |
| expect(el).toBeInTheDocument(); | |
| return el as HTMLInputElement; | |
| }); | |
| fireEvent.change(input, { target: { value: "New Value" } }); | |
| fireEvent.keyDown(input, { key: "Enter", code: "Enter" }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/client/src/components/editorComponents/EditableText.test.tsx` around
lines 122 - 130, The waitFor callback must not perform side-effectful
interactions; instead, use waitFor to assert the input exists (e.g., waitFor(()
=> expect(document.querySelector(".bp3-editable-text-input")).toBeTruthy())) and
then do the fireEvent.change and fireEvent.keyDown on the selected element
outside of waitFor. Update the occurrences that use
document.querySelector(".bp3-editable-text-input") in EditableText.test.tsx (the
blocks around the current fireEvent calls) to first waitFor/assert the element
is present, then perform the change/Enter key events after that assertion; apply
this same pattern to the other listed occurrences so no fireEvent calls are
executed inside waitFor.
Comment on lines
+35
to
+37
| 1. Drag a Button widget from the widget panel onto your canvas | ||
| 2. Configure the **Label** property to set the button text | ||
| 3. Add an **onClick** action to define what happens when clicked |
Contributor
There was a problem hiding this comment.
Unify button text property naming (Label vs text) across the doc.
Line 36 uses Label, while Line 53 and multiple examples use text. Please standardize on one canonical property name to avoid misconfiguration from copy-paste.
Also applies to: 53-54, 223-224, 321-322
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/client/src/widgets/ButtonWidget/README.md` around lines 35 - 37, The
README for the Button widget uses two different property names for the button
caption (Label vs text); standardize on a single canonical property (choose
either Label or text) throughout the document and examples to avoid confusion:
update every instance of "Label" and all examples that reference `text` so they
use the chosen property consistently (search for "Label", "`text`", "Button
widget", and the examples around lines referenced in the review) and ensure any
code snippets, docs, and example usages use the same property name and
description.
Comment on lines
+41
to
+45
| ``` | ||
| // Button configuration | ||
| Label: "Submit Form" | ||
| onClick: {{ Api1.run() }} | ||
| ``` |
Contributor
There was a problem hiding this comment.
Add language identifiers to fenced code blocks to satisfy markdownlint (MD040).
The blocks at Line 41 and Line 162 should declare a language (for example, text or javascript).
Proposed doc fix
-```
+```text
// Button configuration
Label: "Submit Form"
onClick: {{ Api1.run() }}- +text
✅ Good: "Save Changes", "Submit Order", "Delete Item"
❌ Avoid: "Click Here", "Submit", "OK"
Also applies to: 162-165
🧰 Tools
🪛 markdownlint-cli2 (0.22.0)
[warning] 41-41: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/client/src/widgets/ButtonWidget/README.md` around lines 41 - 45, Two
fenced code blocks in the ButtonWidget README examples are missing language
identifiers, triggering markdownlint MD040; update the code fences in README.md
for the ButtonWidget examples to include a language tag (e.g., use ```text or
```javascript) so both the configuration snippet (Label/onClick example) and the
button label guidance block declare their language; locate the fenced blocks in
the ButtonWidget README and prefix the opening ``` with the chosen language
identifier for each block.
| | Property | Type | Default | Description | | ||
| |----------|------|---------|-------------| | ||
| | `text` | string | `"Submit"` | The label displayed on the button | | ||
| | `onClick` | action | - | Action to execute when the button is clicked. Can be an API call, query, or JavaScript function | |
Contributor
There was a problem hiding this comment.
Fix sentence fragment in onClick description.
Line 54 currently reads as a fragment after a period. Make it a single complete sentence for clarity and consistency.
Proposed doc fix
-| `onClick` | action | - | Action to execute when the button is clicked. Can be an API call, query, or JavaScript function |
+| `onClick` | action | - | Action to execute when the button is clicked; it can be an API call, query, or JavaScript function |📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| | `onClick` | action | - | Action to execute when the button is clicked. Can be an API call, query, or JavaScript function | | |
| | `onClick` | action | - | Action to execute when the button is clicked; it can be an API call, query, or JavaScript function | |
🧰 Tools
🪛 LanguageTool
[style] ~54-~54: To form a complete sentence, be sure to include a subject or ‘there’.
Context: ... to execute when the button is clicked. Can be an API call, query, or JavaScript fu...
(MISSING_IT_THERE)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/client/src/widgets/ButtonWidget/README.md` at line 54, Update the
`onClick` prop description in ButtonWidget README so it is a single complete
sentence: replace the fragment "Action to execute when the button is clicked.
Can be an API call, query, or JavaScript function" with a single sentence such
as "Action to execute when the button is clicked; it can be an API call, query,
or JavaScript function." Ensure you edit the `onClick` row in the ButtonWidget
README (`onClick` prop description) to reflect this wording.
Comment on lines
+363
to
+368
| text: {{ TableRow.isSelected ? "Edit Selected" : "Select an Item" }} | ||
| isDisabled: {{ !TableRow.isSelected }} | ||
| buttonVariant: {{ TableRow.isSelected ? "PRIMARY" : "SECONDARY" }} | ||
| onClick: {{ | ||
| navigateTo('EditPage', { id: TableRow.selectedRow.id }); | ||
| }} |
Contributor
There was a problem hiding this comment.
Fix invalid widget reference in Example 3 (TableRow likely should be a concrete widget name).
This sample uses TableRow.isSelected and TableRow.selectedRow.id, but other examples use concrete widget references (for example, Table1). As written, this snippet is likely not executable.
Proposed doc fix
-text: {{ TableRow.isSelected ? "Edit Selected" : "Select an Item" }}
-isDisabled: {{ !TableRow.isSelected }}
-buttonVariant: {{ TableRow.isSelected ? "PRIMARY" : "SECONDARY" }}
+text: {{ Table1.selectedRow ? "Edit Selected" : "Select an Item" }}
+isDisabled: {{ !Table1.selectedRow }}
+buttonVariant: {{ Table1.selectedRow ? "PRIMARY" : "SECONDARY" }}
onClick: {{
- navigateTo('EditPage', { id: TableRow.selectedRow.id });
+ navigateTo('EditPage', { id: Table1.selectedRow.id });
}}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| text: {{ TableRow.isSelected ? "Edit Selected" : "Select an Item" }} | |
| isDisabled: {{ !TableRow.isSelected }} | |
| buttonVariant: {{ TableRow.isSelected ? "PRIMARY" : "SECONDARY" }} | |
| onClick: {{ | |
| navigateTo('EditPage', { id: TableRow.selectedRow.id }); | |
| }} | |
| text: {{ Table1.selectedRow ? "Edit Selected" : "Select an Item" }} | |
| isDisabled: {{ !Table1.selectedRow }} | |
| buttonVariant: {{ Table1.selectedRow ? "PRIMARY" : "SECONDARY" }} | |
| onClick: {{ | |
| navigateTo('EditPage', { id: Table1.selectedRow.id }); | |
| }} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/client/src/widgets/ButtonWidget/README.md` around lines 363 - 368, The
example references a non-existent generic widget name TableRow; replace those
references with the actual table widget used in other examples (e.g., Table1)
and use its concrete selection properties (e.g., Table1.isSelected or
Table1.selectedRow and Table1.selectedRow.id) so the bindings become executable:
update the expressions in text, isDisabled, buttonVariant and onClick to
reference the concrete widget name and its selectedRow.id and selection flag
instead of TableRow.
|
|
||
| --- | ||
|
|
||
| For more information about contributing to Appsmith, see the [Contributing Guide](../../../../CONTRIBUTING.md). No newline at end of file |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Repo CONTRIBUTING candidates:"
fd -a '^CONTRIBUTING\.md$'
echo
echo "README location:"
fd -a '^README\.md$' | rg 'app/client/src/widgets/ButtonWidget/README.md' || true
echo
echo "Resolved target for current relative link:"
python - <<'PY'
from pathlib import Path
readme = Path("app/client/src/widgets/ButtonWidget/README.md").resolve()
target = (readme.parent / "../../../../CONTRIBUTING.md").resolve()
print(target)
print("exists:", target.exists())
PYRepository: appsmithorg/appsmith
Length of output: 308
Fix relative path to CONTRIBUTING.md.
The current path ../../../../CONTRIBUTING.md resolves to app/CONTRIBUTING.md, which doesn't exist. Use ../../../../../CONTRIBUTING.md instead to correctly reference the CONTRIBUTING.md file at the repository root.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/client/src/widgets/ButtonWidget/README.md` at line 396, Update the
relative link in the ButtonWidget README: change the CONTRIBUTING.md path from
../../../../CONTRIBUTING.md to ../../../../../CONTRIBUTING.md so the link
resolves to the repository root; locate the markdown line that reads "For more
information about contributing to Appsmith, see the [Contributing
Guide](../../../../CONTRIBUTING.md)." and replace the path accordingly.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR adds comprehensive documentation for the Button widget, one of the most commonly used widgets in Appsmith.
Changes
app/client/src/widgets/ButtonWidget/Documentation Structure
The README includes:
Benefits
Testing
Checklist
Summary by CodeRabbit
New Features
isRequiredproperty to Select componentAccessibility