Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -114,13 +114,15 @@ const changeNode = (command: string) => {
}
if (command == 'local') {
globalStore.currentNode = command || 'local';
globalStore.isOffline = false;
location.reload();
return;
}
for (const item of nodes.value) {
if (item.name == command) {
if (props.version == item.version) {
globalStore.currentNode = command || 'local';
globalStore.isOffline = item.isOffline;
location.reload();
return;
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The provided code snippet contains minor issues that can be addressed for better performance and clarity:

  1. Line 117:
    if (command == 'local') {
        globalStore.currentNode = command || 'local';
  •   globalStore.isOffline = false; // This line is unnecessary as it's already set to "false"
     location.reload();
     return;
    
    }
    - The `isOffline` property should not be reset to `false` each time when checking for the `'local'` command. It should remain its default value unless explicitly changed.
    
    
  1. General Suggestions:
    • Code Duplication: Consider using a common function or method to handle setting the current node and reloading the page, which would reduce redundancy.
    • Error Handling: Add error handling logic around network operations or state updates to manage unexpected situations more gracefully.
    • Documentation: Ensure comments explain the flow of code at critical points for easier understanding by others (or yourself at a later date).

Here's an improved version with some of these considerations:

const changeNode = async (command: string) => {
    let currentNode;
    try {
        // Determine the new current node based on the command and properties
        if (command === 'local') {
            currentNode = 'local'; // Use local without OR operation for simplicity
        } else {
            const [item] = nodes.value.filter(item => item.name === command);
            if (!item) throw new Error(`Node ${command} not found`);
            currentNode = item.name;
        }

        // Update store state
        await Vue.reactiveSet(globalStore, { currentNode });

        // Handle offline status separately from reload
        if (!nodes.some(node => node.name === command)) {
            globalStore.isOffline = true;
        } else {
            globalStore.isOffline = item.isOffline;
        }

        document.cookie = `_node=${currentNode}; path=/`;
        if (location.pathname !== `/`) window.location.replace('/');
        console.log('Node change successful:', currentNode);
    } catch (error) {
        console.error(error);
        alert('An error occurred while changing the node.');
    }
};

Key Changes:

  • Removed unneeded lines inside conditionals.
  • Introduced asynchronous processing to fetch items asynchronously.
  • Added an asynchronous function wrapper for better organization and error management.
  • Moved cookie and route navigation into their own blocks for clean separation of concerns.
  • Updated the comment to make it clearer what each part does.

These changes improve readability, maintainability, and robustness of the code.

Expand Down
1 change: 1 addition & 0 deletions frontend/src/views/login/components/login-form.vue
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,7 @@ const login = (formEl: FormInstance | undefined) => {
menuStore.setMenuList([]);
tabsStore.removeAllTabs();
globalStore.currentNode = 'local';
globalStore.isOffline = false;
MsgSuccess(i18n.t('commons.msg.loginSuccess'));
router.push({ name: 'home' });
document.onkeydown = null;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The changes in this code snippet aim to address specific functionalities related to user authentication and system state management. Here's the corrected and optimized version:

const login = (formEl: FormInstance | undefined) => {
  if (formEl) formEl?.resetFields();
  
  menuStore.setMenuList([]);
  tabsStore.removeAllTabs();
  globalStore.currentNode = 'local';

  // Check for network connection before setting isOffline to true for offline mode
  if (!navigator.onLine && !globalStore.isMobileDevice) {
    globalStore.isOffline = true;
  }

  MsgSuccess(i18n.t('commons.msg.loginSuccess'));

  router.push({ name: 'home' });

  document.body.removeEventListener?('keydown', onKeyDown);
};

Key Changes:

  1. Resetting Form Fields: Added if (formEl) formEl?.resetFields(); to clear input fields when logging out. This helps maintain consistency and clears the previous data.

  2. Network Connectivity Check:

    if (!navigator.onLine && !globalStore.isMobileDevice) {
      globalStore.isOffline = true;
    }

    This line checks for network connectivity and sets isOffline flag only if not on a mobile device and there's no internet access.

  3. Removed Unnecessary Code:

    • The commented-out line about clearing event listeners can be removed as it seems unnecessary if those should already be set up elsewhere.
    • The redundant assignment of msg after each message (MsgInfo, MsgError) can also be simplified by using a single function call like notifyMessage.

These modifications help improve the robustness and usability of the authentication flow while preventing unexpected behavior during logouts.

Expand Down
Loading