Skip to content

Add preference to disable table editor shortcuts and migrate preferences to localStorage #1901

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

Merged
merged 4 commits into from
May 15, 2025

Conversation

Yukaii
Copy link
Member

@Yukaii Yukaii commented Mar 21, 2025

Introduce a new preference option to disable shortcuts in the table editor and migrate user preferences from cookies to localStorage for better management.

@Yukaii Yukaii added this to the Next milestone Mar 21, 2025
Signed-off-by: Yukai Huang <[email protected]>
@Yukaii Yukaii force-pushed the feature/dev-1881-shortcut-options branch 2 times, most recently from 2677d21 to 282ba60 Compare March 21, 2025 08:55
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces a new user preference to disable table editor shortcuts and migrates existing user preferences from cookies to localStorage for improved management.

  • Added a method to toggle table editor shortcuts in the table editor module.
  • Updated the statusbar UI to include a checkbox for disabling table editor shortcuts.
  • Implemented a Storage utility and a migration routine in the main index file to transition preference storage from cookies to localStorage.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
public/js/lib/editor/table-editor.js Adds a new API to enable/disable shortcuts and checks the saved cookie value.
public/js/lib/editor/statusbar.html Updates the dropdown menu to include a control for table editor shortcuts.
public/js/lib/editor/index.js Implements a Storage utility and migrates preferences from cookies to localStorage.

@Yukaii Yukaii modified the milestones: Next, 2.6.0 May 8, 2025
@Yukaii Yukaii force-pushed the feature/dev-1881-shortcut-options branch from 1b84a39 to 81611f2 Compare May 15, 2025 06:02
@Yukaii Yukaii requested review from jackycute and Copilot May 15, 2025 06:03
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR introduces a new user preference to disable table editor shortcuts and migrates user preferences from cookies to localStorage to improve manageability.

  • Adds a new method in the table editor to toggle keyboard shortcuts based on user preference.
  • Adds a new checkbox in the status bar to disable table editor shortcuts.
  • Implements a Storage utility and updates preference handling in the Editor class to use localStorage.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
public/js/lib/editor/table-editor.js Added setShortcutsEnabled to enable/disable keymap based on preference
public/js/lib/editor/statusbar.html Inserted a new checkbox for disabling table editor shortcuts
public/js/lib/editor/index.js Introduced Storage utility, migrated preference keys from cookies to localStorage, and updated related preference operations
Comments suppressed due to low confidence (1)

public/js/lib/editor/table-editor.js:136

  • Consider verifying that 'lastActive' is defined in every relevant context to avoid potential runtime errors if it's undefined.
if (!enabled && lastActive) {

Comment on lines +249 to +250
Storage.set(key, cookieValue)
console.log(`Migrated preference ${key} from cookies to localStorage`)
Copy link
Preview

Copilot AI May 15, 2025

Choose a reason for hiding this comment

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

[nitpick] Consider removing each migrated cookie after a successful transfer to localStorage to avoid retaining stale data.

Suggested change
Storage.set(key, cookieValue)
console.log(`Migrated preference ${key} from cookies to localStorage`)
if (Storage.set(key, cookieValue)) {
// Remove the cookie after successful migration
Cookies.remove(key)
console.log(`Migrated and removed cookie for preference ${key} from cookies to localStorage`)
}

Copilot uses AI. Check for mistakes.

@Yukaii Yukaii merged commit 1e20bc1 into develop May 15, 2025
6 checks passed
@Yukaii Yukaii deleted the feature/dev-1881-shortcut-options branch May 15, 2025 06:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants