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

Tornjak dashboard UI- Front End[Part 1] #76

Merged
merged 27 commits into from
Jul 28, 2021

Conversation

mamy-CS
Copy link
Collaborator

@mamy-CS mamy-CS commented Jul 13, 2021

@mamy-CS mamy-CS self-assigned this Jul 13, 2021
@mamy-CS mamy-CS linked an issue Jul 13, 2021 that may be closed by this pull request
4 tasks
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/charts/PieChart.js Outdated Show resolved Hide resolved
tornjak-frontend/src/charts/PieChart.js Outdated Show resolved Hide resolved
tornjak-frontend/src/components/Dashboard/Title.js Outdated Show resolved Hide resolved
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

clusterList() {
if (typeof this.props.globalClustersList !== 'undefined') {
return this.props.globalClustersList.map(currentCluster => {
Copy link
Owner

Choose a reason for hiding this comment

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

I think this.props.globalClustersList.map(this.cluster) should work? give it a go.. same with other patterns like this where the lambda is just return f(x)

Copy link
Collaborator

Choose a reason for hiding this comment

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

so we can simplifying by doing this.props.globalClustersList.map(a => this.cluster(a)), but if we just do this.props.globalClustersList.map(this.cluster), there is an error because the this.cluster function uses function this.numberClusterEntries, and it doesn't recognize this.numberClusterEntries as a function

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.

Awesome - just minor nits and then LGTM

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.

LGTM, thanks for the great feature @mamy-CS @maia-iyer

@lumjjb lumjjb merged commit 2df0d1e into lumjjb:main Jul 28, 2021
@maia-iyer maia-iyer linked an issue Aug 3, 2021 that may be closed by this pull request
3 tasks
lumjjb pushed a commit that referenced this pull request Aug 9, 2021
* tornjak dashboard design details

* fix rendering bugs

* fix navbar render bog on resize

* taking care of initial review nits

* removing title component

* readding title component

* dashboard data management

* cluster edit update

* expire time date reformat

* no data to show display for banner

* fixing ===

* material Ui version update

* merging changes from dashboard data management pr

* nits from review

* remove Dashboard folder

* renaming as dashboard lowercase

* cluster create form cluster types bug fix

* manager mode cluster create type bug fix

* expanded view tables for dashboard

* expanded view refactor tables dashboard

* removing unused variables and functions

* merging from dashboard frontend bug fixing pr

* removing agent update gitignore

* remove agent file

* nits in spiffe-entry-interface

Signed-off-by: Mohammed Abdi <mohammed.munir.abdi@ibm.com>
lumjjb pushed a commit that referenced this pull request Aug 9, 2021
* tornjak dashboard design details

* fix rendering bugs

* fix navbar render bog on resize

* taking care of initial review nits

* removing title component

* readding title component

* dashboard data management

* cluster edit update

* expire time date reformat

* no data to show display for banner

* fixing ===

* material Ui version update

* merging changes from dashboard data management pr

* nits from review

* remove Dashboard folder

* renaming as dashboard lowercase

* cluster create form cluster types bug fix

* manager mode cluster create type bug fix

* expanded view tables for dashboard

* expanded view refactor tables dashboard

* removing unused variables and functions

* merging from dashboard frontend bug fixing pr

* removing agent update gitignore

* remove agent file

* nits in spiffe-entry-interface

Signed-off-by: Mohammed Abdi <mohammed.munir.abdi@ibm.com>
lumjjb pushed a commit that referenced this pull request Aug 9, 2021
* tornjak dashboard design details

* fix rendering bugs

* fix navbar render bog on resize

* taking care of initial review nits

* removing title component

* readding title component

* dashboard data management

* cluster edit update

* expire time date reformat

* no data to show display for banner

* fixing ===

* material Ui version update

* merging changes from dashboard data management pr

* nits from review

* remove Dashboard folder

* renaming as dashboard lowercase

* cluster create form cluster types bug fix

* manager mode cluster create type bug fix

* expanded view tables for dashboard

* expanded view refactor tables dashboard

* removing unused variables and functions

* merging from dashboard frontend bug fixing pr

* removing agent update gitignore

* remove agent file

* nits in spiffe-entry-interface

Signed-off-by: Mohammed Abdi <mohammed.munir.abdi@ibm.com>
lumjjb pushed a commit that referenced this pull request Aug 9, 2021
* tornjak dashboard design details

* fix rendering bugs

* fix navbar render bog on resize

* taking care of initial review nits

* removing title component

* readding title component

* dashboard data management

* cluster edit update

* expire time date reformat

* no data to show display for banner

* fixing ===

* material Ui version update

* merging changes from dashboard data management pr

* nits from review

* remove Dashboard folder

* renaming as dashboard lowercase

* cluster create form cluster types bug fix

* manager mode cluster create type bug fix

* expanded view tables for dashboard

* expanded view refactor tables dashboard

* removing unused variables and functions

* merging from dashboard frontend bug fixing pr

* removing agent update gitignore

* remove agent file

* nits in spiffe-entry-interface

Signed-off-by: Mohammed Abdi <48661743+mamy-CS@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature: Dashboard for organizational identity view Implement dashboard frontend [Part1]
3 participants