Skip to content

refactor: enhance sidebar and page components with tooltips for bulk selection#98

Merged
KazeKaze93 merged 1 commit intomasterfrom
polish/nav-ux
Apr 28, 2026
Merged

refactor: enhance sidebar and page components with tooltips for bulk selection#98
KazeKaze93 merged 1 commit intomasterfrom
polish/nav-ux

Conversation

@KazeKaze93
Copy link
Copy Markdown
Owner

  • Added Tooltip functionality to the bulk selection buttons in the Favorites, Browse, PlaylistsPage, and Updates components, improving user experience by providing contextual information.
  • Adjusted Sidebar layout for better responsiveness and visual consistency.
  • Cleaned up unused imports and ensured adherence to strict coding standards throughout the changes.

…selection

- Added Tooltip functionality to the bulk selection buttons in the Favorites, Browse, PlaylistsPage, and Updates components, improving user experience by providing contextual information.
- Adjusted Sidebar layout for better responsiveness and visual consistency.
- Cleaned up unused imports and ensured adherence to strict coding standards throughout the changes.
@KazeKaze93 KazeKaze93 merged commit b1e3431 into master Apr 28, 2026
1 of 2 checks passed
@github-actions
Copy link
Copy Markdown

🛡️ Архитектор (AI Review)

Этот PR — классический пример того, как «улучшение UX» превращается в мусорную свалку из дублирующегося кода. Ты размазал одну и ту же логику по четырем разным страницам. Как архитектор, я не могу на это смотреть без боли.

⚠️ Надо исправить

1. Нарушение принципа DRY (Don't Repeat Yourself)
Ты скопировал один и тот же блок кода с Tooltip, Button и логикой переключения в Browse.tsx, Favorites.tsx, PlaylistsPage.tsx и Updates.tsx. Если завтра изменится дизайн кнопки или текст тултипа, ты будешь править это в четырех местах?
Решение: Создай отдельный компонент BulkModeToggle.tsx.

2. Избыточность TooltipProvider
Зачем ты оборачиваешь каждую кнопку в TooltipProvider? Этот компонент должен находиться на верхнем уровне приложения (в App.tsx или Layout.tsx), чтобы управлять состоянием всех тултипов сразу. Пихать его вглубь страниц — это лишние маунты в React-дереве.

3. Примитивная логика переключения
Твой onClick перегружен лишними условиями.

// Было
onClick={() => {
  if (isBulkMode) {
    deactivateBulkMode();
    return;
  }
  activateBulkMode();
}}

// Надо (внутри твоего будущего компонента)
const toggleBulkMode = () => isBulkMode ? deactivateBulkMode() : activateBulkMode();

💡 Совет

1. Вынеси компонент
Создай src/renderer/components/BulkModeToggle.tsx:

import { CheckSquare } from "lucide-react";
import { Button } from "./ui/button";
import { Tooltip, TooltipContent, TooltipTrigger } from "./ui/tooltip";

interface Props {
  isBulkMode: boolean;
  activate: () => void;
  deactivate: () => void;
}

export const BulkModeToggle = ({ isBulkMode, activate, deactivate }: Props) => (
  <Tooltip>
    <TooltipTrigger asChild>
      <Button
        type="button"
        variant={isBulkMode ? "default" : "outline"}
        size="icon"
        aria-label="Toggle bulk selection mode"
        onClick={() => (isBulkMode ? deactivate() : activate())}
      >
        <CheckSquare className="h-4 w-4" />
      </Button>
    </TooltipTrigger>
    <TooltipContent>Bulk selection</TooltipContent>
  </Tooltip>
);

2. Очисти страницы
В Browse.tsx и остальных файлах просто используй:

<BulkModeToggle 
  isBulkMode={isBulkMode} 
  activate={activateBulkMode} 
  deactivate={deactivateBulkMode} 
/>

3. Глобальный Provider
Удали TooltipProvider из страниц. Перенеси его в корень приложения, чтобы не плодить сущности.

Итог: Код рабочий, но архитектурно — это «костыльная фабрика». Исправляй дублирование, пока оно не пустило корни. 🟢 LGTM только после рефакторинга в общий компонент.

Analyzed by: gemini-3-flash-preview

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.

1 participant