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.

Sign up for GitHub

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

Jump to bottom

Tornjak dashboard details metadata and UI [Part2] #95

Open
wants to merge 19 commits into
base: main-old
Choose a base branch
from
Open

Tornjak dashboard details metadata and UI [Part2] #95

wants to merge 19 commits into from

Conversation

mamy-CS
Copy link
Collaborator

@mamy-CS mamy-CS commented Aug 4, 2021

@mamy-CS mamy-CS self-assigned this Aug 4, 2021
@lumjjb
Copy link
Owner

lumjjb commented Aug 5, 2021

Looking good - a few notes about the feature that I'd like to add for discussion

  • If it's easier feel free to simplify it so that it only selects 1 entity at a time. Can be pretty tricky to deal with multiple objects. This I feel would be very useful to start for a user.
  • A thing to note with these "details" pages is that we probably want to be able to share links to these things.. so it may be worth exploring this from an angle of defining what the URL would look like (with parameters), and figuring out how to construct the page from there.
  • Let's discuss this tomorrow, but as of now the pages are similar enough that they'd share most components, however, it is likely with future consideration that these pages would look very little like each other... So having a separate page for viewDetails may be something to consider.

lumjjb
lumjjb reviewed Aug 10, 2021
View reviewed changes
Copy link
Owner

@lumjjb lumjjb left a comment

Choose a reason for hiding this comment

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

Initial review

tornjak-frontend/src/App.js Outdated Show resolved Hide resolved
tornjak-frontend/src/components/dashboard/agents-dashboard-table.js Outdated Show resolved Hide resolved
tornjak-frontend/src/components/dashboard/dashboard-details.js Outdated
<Container maxWidth="lg" className={classes.container}>
<Grid item xs={12}>
<Paper className={classes.paper}>
<p className="details-title">Agent Name : <b>{this.props.globalSelectedDashboardData[0].value.spiffeid}</b></p>
Copy link
Owner

Choose a reason for hiding this comment

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

is it possible to make the bold formatting of text tied to the CSS of the

tag?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

What do you mean by this?

tornjak-frontend/src/components/dashboard/dashboard-details.js Show resolved Hide resolved
tornjak-frontend/src/components/dashboard/agents-dashboard-table.js Outdated Show resolved Hide resolved
tornjak-frontend/src/components/dashboard/dashboard-drawer.js Show resolved Hide resolved
tornjak-frontend/src/components/dashboard/table/dashboard-table.js Outdated Show resolved Hide resolved
tornjak-frontend/src/components/dashboard/table/dashboard-table.js Outdated Show resolved Hide resolved
tornjak-frontend/src/components/dashboard/table/dashboard-table.js Outdated Show resolved Hide resolved
tornjak-frontend/src/redux/reducers/tornjakReducer.js Outdated Show resolved Hide resolved
@lumjjb
Copy link
Owner

lumjjb commented Aug 18, 2021

i am unable to select details, there's no checkbox available.
Screen Shot 2021-08-18 at 10 32 14 AM

@mamy-CS mamy-CS mentioned this pull request Aug 23, 2021
Merged
@mamy-CS mamy-CS mentioned this pull request Sep 17, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lumjjb lumjjb lumjjb left review comments

Assignees

@mamy-CS mamy-CS

Labels
Status: Ready for Review 👀
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mamy-CS @lumjjb