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

CB-6128 add tooltips to tabs #3221

Open
wants to merge 7 commits into
base: devel
Choose a base branch
from
Open

CB-6128 add tooltips to tabs #3221

wants to merge 7 commits into from

Conversation

devnaumov
Copy link
Member

@devnaumov devnaumov commented Jan 28, 2025

useService is used instead of useResource intentionally. It will decrease amount of calls to the resources. We dont need to force data to be presented (connection and project core entities).

@devnaumov devnaumov requested review from Wroud and removed request for Wroud January 28, 2025 16:31
Comment on lines 64 to 84
let tooltip = name;

if (connection) {
tooltip += `\n${translate('ui_connection')}: ${connection.name}`;
}

if (executionContext) {
const project = projectInfo.get(executionContext.projectId);

if (executionContext.defaultCatalog) {
tooltip += `\n${translate('ui_catalog')}: ${executionContext.defaultCatalog}`;
}

if (executionContext.defaultSchema) {
tooltip += `\n${translate('ui_schema')}: ${executionContext.defaultSchema}`;
}

if (project) {
tooltip += `\n${translate('ui_project')}: ${project.name}`;
}
}
Copy link
Member

Choose a reason for hiding this comment

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

this part of logic is duplicated, can we move it to separate function-helper that will return tooltip string (localized)
something like getObjectInfoTooltip(translate, connection?, catalog?, schema?, project?)

@devnaumov devnaumov requested a review from Wroud February 5, 2025 16:37

import { useTranslate } from './localization/useTranslate.js';

export function useObjectInfoTooltip(connection?: string, catalog?: string, schema?: string, project?: string) {
Copy link
Contributor

Choose a reason for hiding this comment

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

it is already almost a lot arguments. in such cases better to provide 1 argument as an object so it is easier to handle

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