Skip to content

Fix #8123 #8128

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.

Sign up for GitHub

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

Open
djhvscf wants to merge 5 commits into
base: develop
Choose a base branch
from
Open

Fix #8123 #8128

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
4026556
Fix #8123
djhvscf Jan 3, 2026
7956404
Fix build issue
djhvscf Jan 3, 2026
40829bb
Merge branch '8123' of https://github.com/wenzhixin/bootstrap-table i…
djhvscf Jan 3, 2026
eeebcd4
Fix compilation issues
djhvscf Jan 3, 2026
3f0f7d3
test: add e2e test page for auto-refresh extension and local Claude p…
djhvscf Jan 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions cypress/e2e/extensions/auto-refresh/auto-refresh.cy.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
describe('Auto Refresh', () => {
it('should toggle active state', () => {
cy.visit('cypress/e2e/extensions/auto-refresh/test.html')
cy.get('[name="autoRefresh"]').click()
Comment on lines +3 to +4
Copy link

Copilot AI Jan 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test should verify the initial state of the auto-refresh button. Since the default value of autoRefreshStatus was changed to false, the button should not have the 'active' class when the page first loads. Consider adding an assertion before the first click to verify this initial state.

Copilot uses AI. Check for mistakes.
cy.get('[name="autoRefresh"]').should('have.class', 'active')
cy.get('[name="autoRefresh"]').click()
cy.get('[name="autoRefresh"]').should('not.have.class', 'active')
})
})
43 changes: 43 additions & 0 deletions cypress/e2e/extensions/auto-refresh/test.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
<!DOCTYPE html>
<html lang="en">

<head>
<meta charset="UTF-8">
<title>Auto Refresh Test</title>
<link rel="stylesheet" href="https://cdn.jsdelivr.net/npm/[email protected]/dist/css/bootstrap.min.css">
<link rel="stylesheet" href="../../../../dist/bootstrap-table.min.css">
<link rel="stylesheet" href="https://cdn.jsdelivr.net/npm/[email protected]/font/bootstrap-icons.css">
</head>

<body>
<div class="container">
<table id="table" data-toggle="table" data-pagination="true" data-search="true" data-auto-refresh="true">
<thead>
<tr>
<th data-field="id">ID</th>
<th data-field="name">Name</th>
<th data-field="price">Price</th>
</tr>
</thead>
<tbody>
<tr>
<td>1</td>
<td>Item 1</td>
<td>$1</td>
</tr>
<tr>
<td>2</td>
<td>Item 2</td>
<td>$2</td>
</tr>
</tbody>
</table>
</div>

<script src="https://code.jquery.com/jquery-3.7.1.min.js"></script>
<script src="https://cdn.jsdelivr.net/npm/[email protected]/dist/js/bootstrap.bundle.min.js"></script>
<script src="../../../../dist/bootstrap-table.min.js"></script>
<script src="../../../../src/extensions/auto-refresh/bootstrap-table-auto-refresh.js"></script>
</body>

</html>
24 changes: 15 additions & 9 deletions src/extensions/auto-refresh/bootstrap-table-auto-refresh.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ Object.assign($.fn.bootstrapTable.defaults, {
showAutoRefresh: true,
autoRefreshInterval: 60,
autoRefreshSilent: true,
autoRefreshStatus: true,
autoRefreshStatus: false,
Copy link
Owner

@wenzhixin wenzhixin Jan 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should the default value here be modified?

autoRefreshFunction: null
})

Expand Down Expand Up @@ -42,16 +42,22 @@ $.BootstrapTable = class extends $.BootstrapTable {

initToolbar (...args) {
if (this.options.autoRefresh) {
this.buttons = Object.assign(this.buttons, {
const attributes = {
'aria-label': this.options.formatAutoRefresh(),
title: this.options.formatAutoRefresh()
}

if (this.options.autoRefreshStatus) {
attributes.class = this.constants.classes.buttonActive || 'active'
}

this.buttons = Object.assign(this.buttons || {}, {
Copy link

Copilot AI Jan 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The defensive || {} fallback is unnecessary here. The base class always initializes this.buttons in the initConstants() method (see src/modules/initialization.js line 32-34). This is inconsistent with other extensions like copy-rows, custom-view, export, filter-control, and toolbar which use Object.assign(this.buttons, {...}) without the fallback. While this doesn't cause issues, it's better to maintain consistency across the codebase.

Suggested change
this.buttons = Object.assign(this.buttons || {}, {
Object.assign(this.buttons, {

Copilot uses AI. Check for mistakes.
autoRefresh: {
text: this.options.formatAutoRefresh(),
icon: this.options.icons.autoRefresh,
render: false,
event: this.toggleAutoRefresh,
attributes: {
'aria-label': this.options.formatAutoRefresh(),
title: this.options.formatAutoRefresh()
}
event: this.toggleAutoRefresh.bind(this),
Copy link

Copilot AI Jan 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The .bind(this) call is redundant here. The toolbar module already calls event handlers with the correct context using .call(this) (see src/modules/toolbar.js line 230). While this doesn't cause any issues, it's unnecessary and inconsistent with other extensions like copy-rows, custom-view, and filter-control which don't use .bind().

Suggested change
event: this.toggleAutoRefresh.bind(this),
event: this.toggleAutoRefresh,

Copilot uses AI. Check for mistakes.
attributes
}
})
}
Expand All @@ -63,11 +69,11 @@ $.BootstrapTable = class extends $.BootstrapTable {
if (this.options.autoRefresh) {
if (this.options.autoRefreshStatus) {
clearInterval(this.options.autoRefreshFunction)
this.$toolbar.find('>.columns .auto-refresh')
this.$toolbar.find('>.columns [name="autoRefresh"]')
.removeClass(this.constants.classes.buttonActive)
} else {
this.setupRefreshInterval()
this.$toolbar.find('>.columns .auto-refresh')
this.$toolbar.find('>.columns [name="autoRefresh"]')
.addClass(this.constants.classes.buttonActive)
}
this.options.autoRefreshStatus = !this.options.autoRefreshStatus
Expand Down
Loading