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

Migrate Public Dashboard Page to React #4203

Closed
wants to merge 12 commits into from

Conversation

gabrieldutra
Copy link
Member

@gabrieldutra gabrieldutra commented Oct 1, 2019

What type of PR is this? (check all applicable)

  • Refactor

Description

Migration of the Public Dashboard Page to React

To-do list:

  • Make sure the auto-refresh works
  • Check other features in the page
  • Delete old files

Issues to verify in this PR:

Related Tickets & Documents

--

Mobile & Desktop Screenshots/Recordings (if there are UI changes)

react-public-dashboard

@gabrieldutra gabrieldutra added the Frontend: React Frontend codebase migration to React label Oct 1, 2019
@gabrieldutra gabrieldutra self-assigned this Oct 1, 2019
Comment on lines +149 to 159
return this.queryResult.toPromise()
.then((result) => {
this.loading = false;
this.data = result;
return result;
})
.catch((error) => {
this.loading = false;
this.data = error;
return error;
});
Copy link
Member Author

@gabrieldutra gabrieldutra Oct 3, 2019

Choose a reason for hiding this comment

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

I think this has been playing around with me for a while (in here and in the Widget Migration).

image

It was returning the queryResult before updating this.data, which was a weird result when I expected the widgets to render an error and they were not doing it

Copy link
Member Author

@gabrieldutra gabrieldutra left a comment

Choose a reason for hiding this comment

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

Basically migrated as is, only some changes in code to be more similar to the dashboard code (so I can reuse the methods in there) and took the opportunity to add the missing Dashboard level Filters:

react-public-dashboard

this.dashboard = new Dashboard(data);
this.dashboard.widgets = Dashboard.prepareDashboardWidgets(this.dashboard.widgets);
this.dashboard.widgets.forEach(widget => this.loadWidget(widget, !!refreshRate));
this.filters = []; // TODO: implement (@/services/dashboard.js:collectDashboardFilters)
Copy link
Member Author

Choose a reason for hiding this comment

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

I've added the Dashboard filters in this PR 👍

@gabrieldutra gabrieldutra marked this pull request as ready for review October 3, 2019 22:36
@gabrieldutra
Copy link
Member Author

Closing this to deliver with #4228

@gabrieldutra gabrieldutra deleted the react-public-dashboard branch November 4, 2019 12:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Frontend: React Frontend codebase migration to React
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants