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

Task route hierarchy in the UI #3472

Merged
merged 3 commits into from
Nov 14, 2017
Merged

Task route hierarchy in the UI #3472

merged 3 commits into from
Nov 14, 2017

Conversation

DingoEatingFuzz
Copy link
Contributor

Making room for logs and file browsing.

Updated allocation detail page:
image

New task overview page:
image


model({ name }) {
const allocation = this.modelFor('allocations.allocation');
if (allocation) {
Copy link
Member

Choose a reason for hiding this comment

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

else?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If allocation is falsey, the allocation route model hook will set the application error state.

export default Controller.extend({
network: computed.alias('model.resources.networks.firstObject'),
ports: computed('network.reservedPorts.[]', 'network.dynamicPorts.[]', function() {
const ports = this.get('network.reservedPorts')
Copy link
Member

Choose a reason for hiding this comment

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

can probably just return here

<div class="boxed-section is-small">
<div class="boxed-section-body inline-definitions">
<span class="label">Task Details</span>
<span class="pair">
Copy link
Member

@thrashr888 thrashr888 Nov 3, 2017

Choose a reason for hiding this comment

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

can try dl/dd here instead of .pair, if that makes the css any easier.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe. It would certainly be semantically better.

@@ -105,83 +111,17 @@ test('each task row should list high-level information for the task', function(a
);

assert.ok(reservedPorts.length, 'The task has reserved ports');
assert.ok(dynamicPorts.length, 'The task has reserved ports');
Copy link
Member

Choose a reason for hiding this comment

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

"reserved" -> "dynamic"

Copy link
Member

@thrashr888 thrashr888 left a comment

Choose a reason for hiding this comment

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

A few comments but looks cool otherwise.

Drilling into a task from an allocation transitions to the task
heirarchy.
Now that there is a task detail page, some of the content from
the allocation detail page is better suited there.
@DingoEatingFuzz DingoEatingFuzz merged commit 4de2ad3 into master Nov 14, 2017
@DingoEatingFuzz DingoEatingFuzz deleted the f-ui-task-hierarchy branch November 14, 2017 19:15
@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants