Skip to content
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

edit duplicate variable name #1354

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

natalievalcin
Copy link

✨ Pull Request

📓 Referenced Issue

Fixes: #1327

ℹ️ About the PR

This PR adds a filter dropdown to streamline device selection by type (e.g., Phone, Tablet). Users can now filter devices by type or select "All Device Types" to view everything.
I am not sure if the Search Bar is suppose to remain or not. So currently the search bar is still included and works along side the filter dropdown.

🖼️ Testing Scenarios / Screenshots

AllDevices PhoneDevice

@natalievalcin
Copy link
Author

@violetadev

@CLAassistant
Copy link

CLAassistant commented Jan 16, 2025

CLA assistant check
All committers have signed the CLA.

@@ -11,13 +11,15 @@ import {
import { APP_VIEWS, setAppView } from 'renderer/store/features/ui';
Copy link
Contributor

Choose a reason for hiding this comment

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

thanks for the PR, please follow the following structure:

  • Make a file named DeviceManager.tsx for this file
  • In the index.tsx file just do export { DeviceManager } from './DeviceManager -> this acts as a export hub for anything you need to export
  • Create separate file components for the lines of code I am commenting below

Comment on lines 153 to 164
<div className="flex items-center gap-2">
{filteredType === type && <Icon icon="mdi:check" />}
<span
className={cx('capitalize', {
'font-semibold text-black dark:text-white':
filteredType === type,
})}
>
{type}
</span>
</div>
),
Copy link
Contributor

Choose a reason for hiding this comment

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

This is the same as the lines 137 to 147, please refactor into a reusable separate component. You could call it DeviceManagerLabel if you'd like, and pass props to it to handle variations

- Moved the DeviceManager logic to a separate DeviceManager.tsx file
- Updated imports
- Refactored code as per the PR review into separate DeviceManagerLabel.tsx file
- Ensured AppContent now correctly references the DeviceManager
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants