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

Allow header disclaimer and logo to be skinned #1260

Merged
merged 3 commits into from
Oct 19, 2017

Conversation

jonathaningram
Copy link
Contributor

@jonathaningram jonathaningram commented Oct 12, 2017

If you're interested, see our matching PR that matches this change: govau/ci-console#3

mainFields: ['browserify', 'browser', 'module', 'main']
mainFields: ['browserify', 'browser', 'module', 'main'],

symlinks: false
Copy link
Contributor

Choose a reason for hiding this comment

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

just curious why was this needed?

Copy link
Contributor Author

@jonathaningram jonathaningram Oct 13, 2017

Choose a reason for hiding this comment

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

This allows us to symlink our skin into the skins directory:

$ ls -l static_src/skins/
total 8
0 drwxr-xr-x  3 me  staff  102 13 Oct 09:09 cg
8 lrwxr-xr-x  1 me  staff   56  5 Oct 14:52 govau -> /Users/me/go/src/github.com/govau/ci-console/skin/

And we are symlinking (at the very least) for development: our skin is in another repo and I symlink it up.

Without this change, the UI won't build, giving the error:

ERROR in /Users/me/go/src/github.com/govau/ci-console/skin/index.js
Module build failed: ReferenceError: Unknown plugin "transform-object-rest-spread" specified in "base" at 0, attempted to resolve relative to "/Users/me/go/src/github.com/govau/ci-console/skin"
<omitted>

Upgrading babel and friends could fix this, but this config change does fix it and doesn't appear to affect anything else.

Copy link
Contributor

@jcscottiii jcscottiii Oct 17, 2017

Choose a reason for hiding this comment

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

thanks for that explanation! @jonathaningram could you make an issue to revisit this when babel is upgraded?

@jcscottiii
Copy link
Contributor

@jonathaningram LGTM. just a question

@mogul
Copy link
Contributor

mogul commented Oct 14, 2017

[edit: Never mind, just saw the question you're referencing above in the review.]
What's the question? :)

jcscottiii
jcscottiii previously approved these changes Oct 17, 2017
Copy link
Contributor

@jcscottiii jcscottiii left a comment

Choose a reason for hiding this comment

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

@jonathaningram this is good to go

@jonathaningram
Copy link
Contributor Author

@jcscottiii rebased. Also I updated circle.yml (see 4822c34) to install latest chromedriver. I needed this to fix my failing integration tests for this PR (which worked locally).

This hopefully also fixes flakiness as you can see in the failed master build: https://circleci.com/gh/18F/cg-dashboard/2654

@jcscottiii jcscottiii merged commit a901dce into cloud-gov:master Oct 19, 2017
bleach pushed a commit to bleach/cg-dashboard-skin that referenced this pull request Oct 22, 2017
A series of PRs from AusDTA are creating the concept of a skin within
cg-dashboard

As well as "templates" used by go components, there are js files consumed by
the frontend.

Here's DTA's current skin:
https://github.com/govau/ci-console/tree/535b8fc466e42e20b22e11109c14ad5ee41a3561/skin

Here are the PRs raised so far:

cloud-gov/cg-dashboard#1259 (merged)
cloud-gov/cg-dashboard#1260 (I'm basing my WIP off this)
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.

3 participants