Skip to content
This repository has been archived by the owner on May 19, 2020. It is now read-only.

Allow finer-grained skinning of home/overview page #1262

Merged
merged 2 commits into from
Nov 8, 2017

Conversation

jonathaningram
Copy link
Contributor

This allows skins to provide an array of panels for the home page. Panels are stacked vertically (as they already were) - however this PR just allows skins to provide whatever panels they like. Inside a panel, the skin can do what it wants. The cg skin retains the same behaviour and just renders some content tiles in a grid pattern. Note: I've moved these content tiles to the cg skin folder since they are specifically for cloud.gov.

I also added an import alias named dashboard so that skins can import components from the main dashboard source without having to rely on relative imports like ../../../../components/foo. An example use is:

import Panel from 'dashboard/components/panel.jsx';
import PanelGroup from 'dashboard/components/panel_group.jsx';

You can see cloud.gov.au's PR that matches this change: https://github.com/govau/ci-console/pull/1/files

I didn't want to make any more PRs until the prettier one is merged so I can avoid merge conflicts, but I wanted to get a start on this one.

@jonathaningram jonathaningram force-pushed the ji-home-skinning branch 2 times, most recently from 65df341 to 2019dec Compare October 24, 2017 09:50
@jonathaningram
Copy link
Contributor Author

@jcscottiii ready for review.

@jcscottiii
Copy link
Contributor

@jonathaningram i think a new .md doc for how to do skins should be created. that way someone in the future won't have to find this PR to do the same thing.

@jonathaningram
Copy link
Contributor Author

@jonathaningram i think a new .md doc for how to do skins should be created. that way someone in the future won't have to find this PR to do the same thing.

@jcscottiii how about instead of writing a document, we include a "default" skin in this project? Here's my thinking:

I don't like writing documentation because I feel it becomes out of date with the code changes that are constantly coming in. Take the skins/cg/index.js doc stanza that I've recently removed. It was just annoying trying to keep it up-to-date. And nothing stopped me from forgetting to update the hard-coded code snippets in there that easily became out-of-date.

If we add a default skin, it will contain real code that must compile. Also, I'm not sure how you feel about this, but it can be the beginnings of a default non-cloud.gov skin that is used by default in this application. Again, we're the only users at the moment, but I read an issue somewhere in here saying that this should be a Cloud Foundry console, not a cloud.gov Cloud Foundry console. A default skin would help achieve that goal and also serve as documentation for how to do skins.

I do agree that guides like "How to create your custom cg-dashboard skin" are really useful, but I'm not sure if it's a good use of time at this stage.

@jcscottiii
Copy link
Contributor

jcscottiii commented Nov 7, 2017

@jonathaningram i like love the idea of a default skin. i would love to move this into the cloudfoundry-community org and having a non cloud.gov skin would be great. now, how much work it would take to create such a skin, i'm not sure.

This keeps the PR changes small and eslint-loader can be removed later.
@jonathaningram
Copy link
Contributor Author

@jonathaningram i like love the idea of a default skin. i would love to move this into the cloudfoundry-community org and having a non cloud.gov skin would be great. now, how much work it would take to create such a skin, i'm not sure.

@jcscottiii Awesome! I have just rebased this if you'd like to merge. Probably best to do the default skin in another PR so we can progress with this as it will be quite a large change.

@jcscottiii jcscottiii merged commit f58c4c6 into cloud-gov:master Nov 8, 2017
@jcscottiii
Copy link
Contributor

@jonathaningram thanks for this!! (i know we wanted #1252 in first but had questions there)

@jonathaningram jonathaningram deleted the ji-home-skinning branch November 8, 2017 21:04
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants