Skip to content

Feature/summary layout #24

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

Open
wants to merge 9 commits into
base: develop
Choose a base branch
from

Conversation

rlrrrrr
Copy link

@rlrrrrr rlrrrrr commented Sep 2, 2024

No description provided.

@rlrrrrr
Copy link
Author

rlrrrrr commented Sep 2, 2024

Summary Layout

@@ -1,6 +1,6 @@
import { useContext } from 'react';
import { YadomsConnectionContext, YadomsConnection } from '@yadoms/shared';
import YButton from './widgets/YButton';
// import YButton from './widgets/YButton';
Copy link
Member

Choose a reason for hiding this comment

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

why is this line commented ?

@@ -16,8 +16,10 @@ export function Home(props: PagesHomeProps) {
<h1>Welcome to PagesHome!</h1>
<p>Socket is {connected ? 'connected' : 'DISCONNECTED'}</p>
<p>Server current time is {serverCurrentTime?.toString()}</p>
{/*
Copy link
Member

Choose a reason for hiding this comment

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

same as before

Comment on lines 37 to 39



Copy link
Member

Choose a reason for hiding this comment

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

remove extrat space

Comment on lines 25 to 36
Window: {color: "#4057dc", size:40},
Linux:{color: "#6f706e", size:40},
MacOs:{color: "#000000", size:40},
DefaultOs:{color: "#6e6e70", size:40},
SqLite:{color: "#66b2fc", size:40},
Postgres:{color: "#3177ff", size:40},
Database:{color: "#f14809", size:40},
DatabaseVersion: {color: "#cec483", size:40},
Power: {color: "#00ff28" ,size:40},
Version: {color: "#873be1", size:40},
Size: {color: "#e52121", size:40},
SQL: {color: "#0048fd", size:40},
Copy link
Member

Choose a reason for hiding this comment

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

could u extract color values into an enum or named colored ( we could use them on other components)

Comment on lines 41 to 71
export function isVersion(version: string) : boolean {
return semver.valid(version) != null;
}



function formatDate(isoDate: string | undefined): string {
const {i18n } = useTranslation();
const locale = i18n.language;
if (!isoDate) {
return '';
}

const year = parseInt(isoDate.slice(0, 4), 10);
const month = parseInt(isoDate.slice(4, 6), 10) - 1;
const day = parseInt(isoDate.slice(6, 8), 10);
const hour = parseInt(isoDate.slice(9, 11), 10);
const minute = parseInt(isoDate.slice(11, 13), 10);
const second = parseInt(isoDate.slice(13, 15), 10);
const millisecond = Math.round(parseFloat("0." + isoDate.split('.')[1]) * 1000);

const date = new Date(year, month, day, hour, minute, second, millisecond);

const options: Intl.DateTimeFormatOptions = {
weekday: 'long', year: 'numeric', month: 'long', day: 'numeric',
hour: 'numeric', minute: 'numeric', second: 'numeric',
};

return date.toLocaleDateString(locale, options);

}
Copy link
Member

Choose a reason for hiding this comment

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

could u please move formatDate and isVersion to shared lib


}

export function Card({ icon, label, content, color, isLoading, children }: {
Copy link
Member

Choose a reason for hiding this comment

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

could u extract this component to another component and create a props object for it's input

Comment on lines 97 to 100
{isLoading ? (
<Skeleton height={20} mt="md" width="80%" radius="sm" />
) : isVersion(content) ? (
<Badge color="pink" variant="light" mt={10}>
Copy link
Member

Choose a reason for hiding this comment

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

All data comes from one Ip and calls onces could create a specific skeleton for all card and move this logic of displaying the loader into the parent caller

</Badge>
) : (
<Text size={{ base: 'xs', sm: 'lg' }}>
{Number.isInteger(content) ? `${content} Octets` : content}
Copy link
Member

Choose a reason for hiding this comment

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

u could create a const that is doing this logic

);
}

export function FeaturesGrid({ systemInformations, children, isLoading }: { systemInformations: SystemInformation[], children: ReactNode, isLoading:boolean }) {
Copy link
Member

Choose a reason for hiding this comment

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

could be also extract and a children component and use props object

const getPlatformIcon = (platform: string | undefined) => {
if (!platform) return <SystemIcon size={customization.DefaultOs.size} color={customization.DefaultOs.color} />;;

if (platform.startsWith('Windows')) {
Copy link
Member

Choose a reason for hiding this comment

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

you could use also else if in here

return <LinuxLogo size={customization.Linux.size} color={customization.Linux.color} />;
} else if (platform.startsWith('MacOs')) {
return <MacOSLogo size={customization.MacOs.size} color={customization.MacOs.color} />;
} else {
Copy link
Member

Choose a reason for hiding this comment

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

no need for the latest else

Comment on lines 167 to 174
switch (engine) {
case 'SQLite':
return <SQLiteLogo size={customization.SqLite.size} color={customization.SqLite.color} />;
case 'Postgres':
return <PostgreSQLLogo size={customization.Postgres.size} color={customization.Postgres.color} />;
default:
return <SQLIcon size={customization.SQL.size} color={customization.SQL.color}/>;
}
Copy link
Member

Choose a reason for hiding this comment

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

did u fortget the operator break ?

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