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

Vtrushin/feature/sh-119 #1852

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
Open

Vtrushin/feature/sh-119 #1852

wants to merge 13 commits into from

Conversation

VasiliyTrue
Copy link
Contributor

No description provided.

@VasiliyTrue VasiliyTrue requested a review from a team as a code owner February 27, 2025 09:01
@VasiliyTrue VasiliyTrue force-pushed the vtrushin/feature/SH-119 branch from b1cf95b to ebf9089 Compare February 27, 2025 11:01

export type PlotKey = string;

export type GroupKey = string;

export type GroupInfo = {
id: GroupKey;
id: string;
Copy link
Contributor

Choose a reason for hiding this comment

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

давай оставим отдельный тип GroupKey для id группы


const id = parts[0] || '';

const x = parts.length > 1 && parts[1] ? Number(parts[1]) : 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

тут можно заюзать кастомный toNumber
const x = toNumber(parts[1], 0);
const y = toNumber(parts[2], 0);
const w = toNumber(parts[3], 1);
const h = toNumber(parts[4], 1);

const h = parts.length > 4 && parts[4] ? Number(parts[4]) : 1;

return {
i: `${groupKey}::${id}`,
Copy link
Contributor

Choose a reason for hiding this comment

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

такой формат идентификатора имеет какой-то смысл?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ты про что конкретно? если про двойное двоеточие, то это универсальный паттерн для разделения неймспейсов, что бы избежать случаи, если одно двоеточие уже присутствует присутсвует в общей строке, например в groupKey или id

Copy link
Contributor

@vauweb vauweb Feb 28, 2025

Choose a reason for hiding this comment

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

я про то зачем groupKey в свойстве объекте где groupKey уже указан и так, при этом в закодированном виде как часть строки

@@ -78,3 +79,54 @@ export const toGroupInfoPrefix = (i: number | string) => `${GET_PARAMS.dashboard
export const toPlotPrefix = (i: number | string) => (i && i !== '0' ? `${GET_PARAMS.plotPrefix}${i}.` : '');
export const toVariablePrefix = (i: number | string) => `${GET_PARAMS.variablePrefix}${i}.`;
export const toVariableValuePrefix = (name: string) => `${GET_PARAMS.variableValuePrefix}.${name}`;

// Compresses layout data into a compact string for URL
// Format: "id.x.y.w.h-id.x.y.w.h-..." where values are in sequential order
Copy link
Contributor

Choose a reason for hiding this comment

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

пока не очень нравиться предложенный формат хранения, хотелось бы обсудить данный вопрос

Copy link
Contributor Author

Choose a reason for hiding this comment

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

давай созвонимся

<PlotView
className={cn(
isDashboardEditAllowed && css.pointerEventsNone,
isNotMobile && 'position-relative overflow-hidden w-100 h-100'
Copy link
Contributor

Choose a reason for hiding this comment

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

overflow-hidden ненадо

Copy link
Contributor Author

Choose a reason for hiding this comment

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

условились в чате что оставляем, пока в твоей таске с виджетами не поправишь

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.

2 participants