-
Notifications
You must be signed in to change notification settings - Fork 14
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
Hana details sites frontend #2278
Conversation
f103e6d
to
5184ef2
Compare
5184ef2
to
8456651
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code apart, it looks good to me Xabi
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey man, great job!
I just left some ideas open, just curious what you think about it.
From my side, the code looks great to me! 👍
import AttributesDetails from './AttributesDetails'; | ||
import ReplicationStatusPill from './ReplicationStatusPill'; | ||
|
||
const getSiteHealth = (srHealthStatus) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think about replacing switch case with object literal as an alternative?
const getSiteHealth = (srHealthStatus) => {
const healthStatus = {
'4': 'passing',
'1': 'critical',
};
return healthStatus[srHealthStatus] || 'unknown';
};
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Honestly, i prefer the switch/case 😝
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good so far. 😄
Description
Depends on: #2277
Pending to rebase it once it is merged
Implement the frontend part of the
sites
improvement in the HANA details view.Check the new style in storybook.
Sneak peak:
@jagabomb I have only added the table header with its object in this PR. Changing the bottom rounded style, details button, new columns, etc are not implemented here.
How was this tested?
Tested with UT, e2e and storybook