Skip to content

Commit

Permalink
Move global namespaces dropdown to list page header
Browse files Browse the repository at this point in the history
Move the global namespaces dropdown from the side nav to the
header of the list pages.

To avoid duplication and to help ensure consistency, introduce
a new ListPageLayout container which is responsible for providing
the common layout and functionality for the list pages.

Moving the namespaces dropdown allows us to resolve a number of
long-standing issues:
- remove code duplication required to redirect to list pages
  when changing namespace from details page as this scenario
  is no longer possible
- a namespaced view will now always contain the namespace
  in the URL, providing consistency as it no longer behaves
  differently when navigating from a namespaced / non-namespaced
  view to a namespaced list. This also means that URLs are
  reliable and can be shared to produce the same view the user
  is currently viewing

It also allows us to remove a large amount of custom styling
in SideNav.scss which was required to style the dropdown to
match the dark theme used in the nav.

One final change that was required to avoid potentially
expensive API calls to build the nav for extensions is the
addition of an optional `namespaced` field (defaults to `true`)
on the kubernetes-resource extensions. This is used to ensure
we generate the correct URL in the side nav (i.e. namespaced /
non-namespaced) regardless of the current URL / globally
selected namespace.
  • Loading branch information
AlanGreene authored and tekton-robot committed Dec 2, 2020
1 parent 33f158a commit 97e025b
Show file tree
Hide file tree
Showing 34 changed files with 332 additions and 827 deletions.
11 changes: 6 additions & 5 deletions docs/extensions.md
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,12 @@ See the [Example: Register a CronJob extension](#example-register-a-cronjob-exte

#### ExtensionSpec

| Variable Name | Type | Required | Default | Description |
|---------------|-------------------|----------|---------|----------------------------------|
| apiVersion | string | Yes | - | Extension resource group |
| name | string | Yes | - | Extension resource name |
| displayname | string | Yes | - | Display name in the Dashboard UI |
| Variable Name | Type | Required | Default | Description |
|---------------|-------------------|----------|---------|------------------------------------------------------------------|
| apiVersion | string | Yes | - | Extension resource group |
| name | string | Yes | - | Extension resource name |
| displayname | string | Yes | - | Display name in the Dashboard UI |
| namespaced | boolean | No | true | Specifies whether the Extension represents a namespaced resource |

### Example: Register a CronJob extension

Expand Down
9 changes: 5 additions & 4 deletions src/api/extensions.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,15 @@ export async function getExtensions({ namespace } = {}) {
);
return extensions.concat(
((resourceExtensions && resourceExtensions.items) || []).map(({ spec }) => {
const { displayname: displayName, name } = spec;
const { displayname: displayName, name, namespaced } = spec;
const [apiGroup, apiVersion] = spec.apiVersion.split('/');
return {
displayName,
name,
apiGroup,
apiVersion,
extensionType: 'kubernetes-resource'
displayName,
extensionType: 'kubernetes-resource',
name,
namespaced
};
})
);
Expand Down
4 changes: 4 additions & 0 deletions src/components/App/_carbon.scss
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ $css--reset: true;
$css--default-type: true;
$css--plex: true;

$feature-flags: (
grid-columns-16: true
);

@import '~@tektoncd/dashboard-components/dist/scss/vars';
@include carbon--theme();

Expand Down
1 change: 1 addition & 0 deletions src/containers/App/App.js
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ if (process.env.I18N_PSEUDO) {
});
}

/* istanbul ignore next */
const ConfigErrorComponent = ({ intl, loadingConfigError }) => {
if (!loadingConfigError) {
return null;
Expand Down
12 changes: 7 additions & 5 deletions src/containers/ClusterTasks/ClusterTasks.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import {
urls
} from '@tektoncd/dashboard-utils';

import { LabelFilter } from '..';
import { ListPageLayout } from '..';
import { fetchClusterTasks } from '../../actions/tasks';
import {
getClusterTasks,
Expand Down Expand Up @@ -137,9 +137,11 @@ export /* istanbul ignore next */ class ClusterTasksContainer extends Component
);
}
return (
<>
<h1>ClusterTasks</h1>
<LabelFilter {...this.props} />
<ListPageLayout
hideNamespacesDropdown
title="ClusterTasks"
{...this.props}
>
<Table
headers={initialHeaders}
rows={clusterTasksFormatted}
Expand All @@ -159,7 +161,7 @@ export /* istanbul ignore next */ class ClusterTasksContainer extends Component
{ kind: 'ClusterTasks' }
)}
/>
</>
</ListPageLayout>
);
}
}
Expand Down
12 changes: 7 additions & 5 deletions src/containers/ClusterTriggerBindings/ClusterTriggerBindings.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import {
import { InlineNotification } from 'carbon-components-react';
import { FormattedDate, Table } from '@tektoncd/dashboard-components';

import { LabelFilter } from '..';
import { ListPageLayout } from '..';
import { fetchClusterTriggerBindings } from '../../actions/clusterTriggerBindings';
import {
getClusterTriggerBindings,
Expand Down Expand Up @@ -101,7 +101,11 @@ export /* istanbul ignore next */ class ClusterTriggerBindings extends Component
);

return (
<>
<ListPageLayout
hideNamespacesDropdown
title="ClusterTriggerBindings"
{...this.props}
>
{error && (
<InlineNotification
kind="error"
Expand All @@ -118,8 +122,6 @@ export /* istanbul ignore next */ class ClusterTriggerBindings extends Component
lowContrast
/>
)}
<h1>ClusterTriggerBindings</h1>
<LabelFilter {...this.props} />
<Table
headers={initialHeaders}
rows={clusterTriggerBindingsFormatted}
Expand All @@ -139,7 +141,7 @@ export /* istanbul ignore next */ class ClusterTriggerBindings extends Component
{ kind: 'ClusterTriggerBindings' }
)}
/>
</>
</ListPageLayout>
);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,8 @@ it('ClusterTriggerBindings renders with no bindings', () => {
errorMessage: null
},
namespaces,
notifications: {}
notifications: {},
properties: {}
});

const { getByText } = renderWithRouter(
Expand Down Expand Up @@ -96,7 +97,8 @@ it('ClusterTriggerBindings renders with one binding', () => {
errorMessage: null
},
namespaces,
notifications: {}
notifications: {},
properties: {}
});

const { queryByText } = renderWithRouter(
Expand Down Expand Up @@ -125,7 +127,8 @@ it('ClusterTriggerBindings can be filtered on a single label filter', async () =
errorMessage: null
},
namespaces,
notifications: {}
notifications: {},
properties: {}
});

const { queryByText, getByTestId, getByText } = renderWithRouter(
Expand Down Expand Up @@ -158,7 +161,8 @@ it('ClusterTriggerBindings renders in loading state', () => {
errorMessage: null
},
namespaces,
notifications: {}
notifications: {},
properties: {}
});

const { queryByText } = renderWithRouter(
Expand Down
8 changes: 3 additions & 5 deletions src/containers/Conditions/Conditions.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import {
import { FormattedDate, Table } from '@tektoncd/dashboard-components';
import { InlineNotification } from 'carbon-components-react';

import { LabelFilter } from '..';
import { ListPageLayout } from '..';
import { fetchConditions } from '../../actions/conditions';
import {
getConditions,
Expand Down Expand Up @@ -131,9 +131,7 @@ export class Conditions extends Component {
}

return (
<>
<h1>Conditions</h1>
<LabelFilter {...this.props} />
<ListPageLayout title="Conditions" {...this.props}>
<Table
headers={headers}
rows={conditionsFormatted}
Expand All @@ -154,7 +152,7 @@ export class Conditions extends Component {
{ kind: 'Conditions', selectedNamespace }
)}
/>
</>
</ListPageLayout>
);
}
}
Expand Down
7 changes: 5 additions & 2 deletions src/containers/Conditions/Conditions.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ const testStore = {
notifications: {
webSocketConnected: false
},
properties: {},
...conditionsTestStore
};

Expand All @@ -86,7 +87,8 @@ describe('Conditions', () => {
const mockTestStore = mockStore({
conditions: { byId: {}, byNamespace: {}, isFetching: true },
...namespacesTestStore,
notifications: {}
notifications: {},
properties: {}
});
const { queryByText } = renderWithRouter(
<Provider store={mockTestStore}>
Expand Down Expand Up @@ -131,7 +133,8 @@ describe('Conditions', () => {
...testStore,
notifications: {
webSocketConnected: true
}
},
propeties: {}
})}
>
<Route
Expand Down
8 changes: 3 additions & 5 deletions src/containers/EventListeners/EventListeners.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ import {
import { InlineNotification } from 'carbon-components-react';
import { FormattedDate, Table } from '@tektoncd/dashboard-components';

import { LabelFilter } from '..';
import { ListPageLayout } from '..';
import { fetchEventListeners } from '../../actions/eventListeners';
import {
getEventListeners,
Expand Down Expand Up @@ -116,7 +116,7 @@ export /* istanbul ignore next */ class EventListeners extends Component {
}));

return (
<>
<ListPageLayout title="EventListeners" {...this.props}>
{error && (
<InlineNotification
kind="error"
Expand All @@ -133,8 +133,6 @@ export /* istanbul ignore next */ class EventListeners extends Component {
lowContrast
/>
)}
<h1>EventListeners</h1>
<LabelFilter {...this.props} />
<Table
headers={initialHeaders}
rows={eventListenersFormatted}
Expand All @@ -155,7 +153,7 @@ export /* istanbul ignore next */ class EventListeners extends Component {
{ kind: 'EventListeners', selectedNamespace }
)}
/>
</>
</ListPageLayout>
);
}
}
Expand Down
12 changes: 8 additions & 4 deletions src/containers/EventListeners/EventListeners.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,8 @@ it('EventListeners renders with no bindings', () => {
errorMessage: null
},
namespaces,
notifications: {}
notifications: {},
properties: {}
});

const { getByText } = renderWithRouter(
Expand Down Expand Up @@ -96,7 +97,8 @@ it('EventListeners renders with one binding', () => {
errorMessage: null
},
namespaces,
notifications: {}
notifications: {},
properties: {}
});

const { queryByText, queryByTitle } = renderWithRouter(
Expand Down Expand Up @@ -126,7 +128,8 @@ it('EventListeners can be filtered on a single label filter', async () => {
errorMessage: null
},
namespaces,
notifications: {}
notifications: {},
properties: {}
});

const { getByTestId, getByText, queryByText } = renderWithRouter(
Expand Down Expand Up @@ -159,7 +162,8 @@ it('EventListeners renders in loading state', () => {
errorMessage: null
},
namespaces,
notifications: {}
notifications: {},
properties: {}
});

const { queryByText } = renderWithRouter(
Expand Down
Loading

0 comments on commit 97e025b

Please sign in to comment.