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

Finish UI portion of ServiceDetail page #214

Merged
merged 5 commits into from
Feb 17, 2017
Merged

Conversation

PtrTeixeira
Copy link
Contributor

Finish off (most of) the visual portion of the ServiceDetail page. I
still need to attach the interactions, including the modals, and I would
like to tool around some more with the request history panel. The
existing pane is filterable and can be paged through, and I would like
to at least get filterable done. Unfortunately, that is probably going
to involve making my own component rather than reusing the UITable
component.

Finish off (most of) the visual portion of the ServiceDetail page.  I
still need to attach the interactions, including the modals, and I would
like to tool around some more with the request history panel. The
existing pane is filterable and can be paged through, and I would like
to at least get filterable done. Unfortunately, that is probably going
to involve making my own component rather than reusing the `UITable`
component.
Attach the modals to remove upstreams, reload configs, and delete the
modal to the service detail page. Still to be done is paginating and
filtering the request history in the 'Recent Requests' panel.
Limit the number of results displayed at a time to 5, and add buttons so
that the user can page through the results. Also add a quick search box
to the table. Neither the paging not the search are nearly as nice as on
the existing system; in particular, the table doesn't have the "Showing
{} from {} (filtered from {})" text on the original, and the search uses
`fuzzy`'s default implementation, which means that the results can seem
a little bit off kilter.
@PtrTeixeira PtrTeixeira changed the title [WIP] Finish UI portion of ServiceDetail page Finish UI portion of ServiceDetail page Feb 14, 2017
);
};

const ButtonContainer = ({editable, serviceJson, upstreams,
Copy link
Member

Choose a reason for hiding this comment

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

might make sense to have this in it's own file since it's a separate component.


// [T], PosInt -> [[T]]
// [1, 2, 3, 4, 5, 6, 7], 2 -> [[1, 2], [3, 4], [5, 6], [7]]
const chunk = (arr, size) => {
Copy link
Member

Choose a reason for hiding this comment

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

rather than having a separate util file in the serviceDetail folder, we should put these in the more global utils file. Seems plenty of these may be reused in components for other pages

@@ -29,3 +31,20 @@ export const ReloadService = buildJsonApiAction(
url: `/state/${serviceId}/reload`,
})
);

export const RemoveUpstreams = buildJsonApiAction(
Copy link
Member

Choose a reason for hiding this comment

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

we should make sure to add this action to the reducers as well, the returned output will be more easily accessible

'REMOVE_UPSTREAMS',
'POST',
(loadBalancerService, upstreams, noValidate, noReload) => ({
url: '/request',
Copy link
Member

Choose a reason for hiding this comment

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

there are several different actions that might submit a request. Maybe it makes sense to use the SubmitRequest action in requests.e6 and let each of the submitting processes handle generating the necessary body?

Feel free to disagree, just trying to think how to best organize what might be very similar calls from different places.

removeUpstreams: upstreams,
},
}),
(serviceId) => serviceId,
Copy link
Member

Choose a reason for hiding this comment

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

It would make more sense to have this keyed on the requestId. When fetching results for the request we are going to have to look it up by that id, not the serviceId

Involved two major changes: migrating the single-page utils code into
the package utils code, and changing how the request submission was
handled. In particular, w/r/t the latter, it had been the case that
there as an action creator in the `api/services.es6` file that was
responsible for sending remove upstreams requests. That has now been
removed, in favor of delegating to the more general action creator in
`api/requests.es6` that can handle any kind of request, and just
creating the request body at the point where it is triggered.
Copy link
Member

@ssalinas ssalinas left a comment

Choose a reason for hiding this comment

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

small nitpick comment, after that and fixing merge conflicts go ahead and merge this to the base react branch 👍

@@ -1,5 +1,7 @@
import { buildApiAction, buildJsonApiAction } from './base';


Copy link
Member

Choose a reason for hiding this comment

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

extra new lines

@PtrTeixeira PtrTeixeira merged commit dd99f39 into react Feb 17, 2017
@PtrTeixeira PtrTeixeira deleted the react-service-detail branch February 17, 2017 14:50
@ssalinas ssalinas modified the milestone: 0.5.0 May 23, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants