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

ShlomiShitrit/issue15 - Add Description #28

Merged
merged 8 commits into from
Oct 13, 2024
Merged

Conversation

ShlomiShitrit
Copy link
Collaborator

@ShlomiShitrit ShlomiShitrit commented Oct 8, 2024

Fixes #15

Images:

image

image

image

@ShlomiShitrit ShlomiShitrit self-assigned this Oct 8, 2024
@ShlomiShitrit ShlomiShitrit added the enhancement New feature or request label Oct 8, 2024
@ShlomiShitrit ShlomiShitrit changed the title ShlomiShitrit/issue15 ShlomiShitrit/issue15 - Add Description Oct 8, 2024
Copy link
Contributor

@nadavWeisler nadavWeisler left a comment

Choose a reason for hiding this comment

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

Melech


const rows: generalObject<strOrNum>[] = [];

[0, 1, 2, 3, 4].forEach((index) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Something is not elegant here? Why not iterate the column instead of [0...5]?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed

{text}
</Typography>
<Box component="div" sx={styles.buttonContainer}>
<Button
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider saving components like that in another element; if you have several buttons you use and set all the parameters every time, it is a way for the typo that can harm consistency in the UI. Still, If you create and use a custom button with all the attributes, you fix it. Don't do that now, but it opens an issue with low priority for generalise ui components like the button.

sx={styles.dataButton}
onClick={() => setOpenData(true)}
>
Display Data Sample
Copy link
Contributor

Choose a reason for hiding this comment

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

In Resources

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

sx={styles.attributeButton}
onClick={() => setOpenAttribute(true)}
>
Display Attributes
Copy link
Contributor

Choose a reason for hiding this comment

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

in resources

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

<TableRow
key={index}
sx={{
"&:last-child td, &:last-child th": {
Copy link
Contributor

Choose a reason for hiding this comment

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

why not in style.ts file?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

Copy link

github-actions bot commented Oct 10, 2024

Coverage report

St.
Category Percentage Covered / Total
🟢 Statements
80.57% (+1.16% 🔼)
759/942
🟢 Branches
80% (+1.74% 🔼)
40/50
🔴 Functions
38.24% (-4.62% 🔻)
13/34
🟢 Lines
80.57% (+1.16% 🔼)
759/942
Show new covered files 🐣
St.
File Statements Branches Functions Lines
🟢
... / Dialog.tsx
100% 100% 33.33% 100%
🟢
... / index.ts
100% 100% 100% 100%
🟢
... / Table.style.ts
100% 100% 100% 100%
🔴
... / Table.tsx
32.56% 100% 0% 32.56%
🟢
... / index.ts
100% 100% 100% 100%
Show files with reduced coverage 🔻
St.
File Statements Branches Functions Lines
🟢
... / Header.tsx
100% 100%
33.33% (-66.67% 🔻)
100%

Test suite run success

26 tests passing in 8 suites.

Report generated by 🧪jest coverage report action from 7728a5b

@ShlomiShitrit ShlomiShitrit merged commit faae8c8 into main Oct 13, 2024
3 checks passed
@ShlomiShitrit ShlomiShitrit deleted the ShlomiShitrit/issue15 branch October 13, 2024 07:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add tool tip indication
2 participants