Adding the styled components to the site to help with theme based styling
Created two theme objects for ecash and stealth colors
Added a theme toggle for use in the development process
Updated the current components to styled components styling using the new themes
Details
Details
- Reviewers
bytesofman - Group Reviewers
Restricted Project - Commits
- rABC3c9f5511693f: [e.cash] Add styled components
- Either run the app locally with npm run dev or generate a preview build with @bot preview-ecash
- Inspect the page to ensure everything functions and looks as before
- click the theme switcher box in the bottom right to toggle between themes
Diff Detail
Diff Detail
- Repository
- rABC Bitcoin ABC
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
| web/e.cash/styles/theme.js | ||
|---|---|---|
| 1 ↗ | (On Diff #39897) | While this is the "official" package, i.e. https://www.npmjs.com/package/styled-theming
Is this module adding value? |
Comment Actions
Ah good call, I didnt realize that was such a dated package. I saw it in an example and it seemed like a succinct way to define the theme object, as you can have the different theme values in the same object...but really at the end of the day doesnt add that much in terms of readability and not worth having another package. Can achieve the same thing with just styled components, just need to make a separate theme object for ecash and stealth
Comment Actions
The test plan should be exact steps for how to verify this diff does what it says it will do.
For example in this case,
- Call preview bot
- Verify web page loads as before
However, it's tricky to really review here because it looks like these new colors are not actually applied anywhere. So we're not really sure what this new package does or if the syntax is applied correctly.
In this case, you should apply these new variable color names throughout all existing CSS in this diff. It will make the diff a bit larger, but this step is necessary to demonstrate that this package is functioning correctly.
Also -- since the goal here is to make it relatively easy for us to change themes in the future, it would be worth creating two themes and two sets of variables. Then the test plan would be something like
Patch the diff to your local machine
npm run dev
View the page on your machine and verify variable color names are carried over, e.g. verify some headline is the right color ro some text is primary
Change the theme in _app.js
npm run dev
Verify the same headline is now some other color
Comment Actions
expanding the scope of the diff to include both themes and applying the themes to the exisiting components
Comment Actions
Updated as discussed. See new description and test plan
I also went ahead with adding a theme toggle to the page. Realized it will be very useful going forward as Im developing the two themes since it's much faster to be able to switch between them to make design decisions, rather than updating the code each time. It will also allow other people to preview the themes without having to build/modify locally
| web/e.cash/components/navbar/index.js | ||
|---|---|---|
| 133 ↗ | (On Diff #39931) | Does that work ? In any case you should keep it consistent and use className="" |
| web/e.cash/components/navbar/index.js | ||
|---|---|---|
| 133 ↗ | (On Diff #39931) | It does work lol, but yeah missed that one thanks. fixed |
Comment Actions
@bot preview-ecash
| web/e.cash/pages/_app.js | ||
|---|---|---|
| 1 ↗ | (On Diff #39931) | Do you need to import React? Is it used here? Have not needed to import React from 'react' for a few versions of React now, but mb Next requires it. |
Comment Actions
Build Bitcoin ABC Diffs / Diff Testing (preview-ecash) failed.
Tail of the build log:
chown -R ${ME} /work
chown -R ${ME} /results
chown -R ${ME} /root/.ccache
}
trap "results" EXIT
export TEAMCITY_VERSION="2019.2.4 (build 72059)"
export BASE_CACHE="/root/abc-depends/cache"
export SDK_ARCHIVE_DIR="/root/abc-depends/osx-sdk"
export SOURCES_PATH="/root/abc-depends/sources"
export SDK_DL_REMOTE="http://ci.fabien.cash"
mkdir -p "/root/abc-depends/cache" "/root/abc-depends/osx-sdk" "/root/abc-depends/sources"
./contrib/teamcity/build-configurations.py "preview-ecash"
EOF
) > run-command.sh
chmod +x run-command.sh
~/infra/docker/docker-run.sh \
-a "-v /home/teamcity/.ccache:/root/.ccache -v /home/teamcity/.abc-depends:/root/abc-depends -v "${RESULTS_DIR}":/results" \
-c run-command.sh /work/run-command.sh \
-r "${REGISTRY}" \
abc-base-image-"${HASH}" ./run-command.sh
[22:36:59] : [Step 1/1] Starting: /home/teamcity/buildAgent/temp/agentTmp/custom_script1853473154738254317
[22:36:59] : [Step 1/1] in directory: /home/teamcity/buildAgent/work/jailed-build
[22:36:59]W: [Step 1/1] + '[' -z preview-ecash ']'
[22:36:59]W: [Step 1/1] + case preview-ecash in
[22:36:59] : [Step 1/1] ~/buildAgent/work/jailed-build/bitcoin-abc ~/buildAgent/work/jailed-build
[22:36:59]W: [Step 1/1] + pushd bitcoin-abc
[22:36:59]W: [Step 1/1] + ./contrib/teamcity/build-configurations.py preview-ecash
[22:36:59]W: [Step 1/1] Traceback (most recent call last):
[22:36:59]W: [Step 1/1] File "/home/teamcity/buildAgent/work/jailed-build/bitcoin-abc/./contrib/teamcity/build-configurations.py", line 672, in <module>
[22:36:59]W: [Step 1/1] main()
[22:36:59]W: [Step 1/1] File "/home/teamcity/buildAgent/work/jailed-build/bitcoin-abc/./contrib/teamcity/build-configurations.py", line 660, in main
[22:36:59]W: [Step 1/1] build_configuration = BuildConfiguration(
[22:36:59]W: [Step 1/1] File "/home/teamcity/buildAgent/work/jailed-build/bitcoin-abc/./contrib/teamcity/build-configurations.py", line 67, in __init__
[22:36:59]W: [Step 1/1] self.load(build_name)
[22:36:59]W: [Step 1/1] File "/home/teamcity/buildAgent/work/jailed-build/bitcoin-abc/./contrib/teamcity/build-configurations.py", line 88, in load
[22:36:59]W: [Step 1/1] raise AssertionError(
[22:36:59]W: [Step 1/1] AssertionError: preview-ecash is not a valid build identifier. Valid identifiers are ['templates', 'builds']
[22:36:59]W: [Step 1/1] + RESULT=1
[22:36:59]W: [Step 1/1] + popd
[22:36:59]W: [Step 1/1] + exit 1
[22:36:59] : [Step 1/1] ~/buildAgent/work/jailed-build
[22:36:59]W: [Step 1/1] Process exited with code 1
[22:36:59]E: [Step 1/1] Process exited with code 1 (Step: Command Line)
[22:36:59]E: [Step 1/1] Step Command Line failed
[22:37:00]E: Ant JUnit report watcher
[22:37:00]E: [Ant JUnit report watcher] No reports found for paths:
[22:37:00]E: [Ant JUnit report watcher] +:results/test_bitcoin.xml
[22:37:00]E: [Ant JUnit report watcher] +:results/**/junit_results*.xml
[22:37:00] : Publishing internal artifacts
[22:37:00] : [Publishing internal artifacts] Publishing 1 file using [ArtifactsCachePublisher]
[22:37:00] : [Publishing internal artifacts] Publishing 1 file using [WebPublisher]
[22:37:00]W: Publishing artifacts (1s)
[22:37:00] : [Publishing artifacts] Collecting files to publish: [+:results/**/junit_results*.xml, +:bitcoin-abc/abc-ci-builds/preview-ecash/gitian-results => preview-ecash.tar.gz, +:bitcoin-abc/abc-ci-builds/preview-ecash/*.log => artifacts.tar.gz]
[22:37:00]W: [Publishing artifacts] Artifacts path 'results/**/junit_results*.xml' not found
[22:37:00]W: [Publishing artifacts] Artifacts path 'bitcoin-abc/abc-ci-builds/preview-ecash/gitian-results' not found
[22:37:00]W: [Publishing artifacts] Artifacts path 'bitcoin-abc/abc-ci-builds/preview-ecash/*.log' not found
[22:37:02] : Build finishedComment Actions
Build Bitcoin ABC Diffs / Diff Testing (preview-e.cash) passed.
Preview is available at http://51.178.130.230:41196 for the next 60 minutes.
Comment Actions
I get a pretty bad flash of unstyled content on the first page load, before stuff is cached (can repeat by doing a hard refresh)
Comment Actions
Remove uneeded React import and fix styling flicker on load by adding next config and _document.js outlined in the nextjs example for styled components
https://github.com/vercel/next.js/tree/canary/examples/with-styled-components
Comment Actions
Build Bitcoin ABC Diffs / Diff Testing (preview-e.cash) passed.
Preview is available at http://51.83.220.62:41694 for the next 60 minutes.
Comment Actions
Build Bitcoin ABC Diffs / Diff Testing (preview-ecash) failed.
Tail of the build log:
chown -R ${ME} /work
chown -R ${ME} /results
chown -R ${ME} /root/.ccache
}
trap "results" EXIT
export TEAMCITY_VERSION="2019.2.4 (build 72059)"
export BASE_CACHE="/root/abc-depends/cache"
export SDK_ARCHIVE_DIR="/root/abc-depends/osx-sdk"
export SOURCES_PATH="/root/abc-depends/sources"
export SDK_DL_REMOTE="http://ci.fabien.cash"
mkdir -p "/root/abc-depends/cache" "/root/abc-depends/osx-sdk" "/root/abc-depends/sources"
./contrib/teamcity/build-configurations.py "preview-ecash"
EOF
) > run-command.sh
chmod +x run-command.sh
~/infra/docker/docker-run.sh \
-a "-v /home/teamcity/.ccache:/root/.ccache -v /home/teamcity/.abc-depends:/root/abc-depends -v "${RESULTS_DIR}":/results" \
-c run-command.sh /work/run-command.sh \
-r "${REGISTRY}" \
abc-base-image-"${HASH}" ./run-command.sh
[17:36:45] : [Step 1/1] Starting: /home/teamcity/buildAgent/temp/agentTmp/custom_script703042084643503162
[17:36:45] : [Step 1/1] in directory: /home/teamcity/buildAgent/work/jailed-build
[17:36:45] : [Step 1/1] ~/buildAgent/work/jailed-build/bitcoin-abc ~/buildAgent/work/jailed-build
[17:36:45]W: [Step 1/1] + '[' -z preview-ecash ']'
[17:36:45]W: [Step 1/1] + case preview-ecash in
[17:36:45]W: [Step 1/1] + pushd bitcoin-abc
[17:36:45]W: [Step 1/1] + ./contrib/teamcity/build-configurations.py preview-ecash
[17:36:46]W: [Step 1/1] Traceback (most recent call last):
[17:36:46]W: [Step 1/1] File "./contrib/teamcity/build-configurations.py", line 672, in <module>
[17:36:46]W: [Step 1/1] main()
[17:36:46]W: [Step 1/1] File "./contrib/teamcity/build-configurations.py", line 661, in main
[17:36:46]W: [Step 1/1] script_dir, config_path, args.build)
[17:36:46]W: [Step 1/1] File "./contrib/teamcity/build-configurations.py", line 67, in __init__
[17:36:46]W: [Step 1/1] self.load(build_name)
[17:36:46]W: [Step 1/1] File "./contrib/teamcity/build-configurations.py", line 90, in load
[17:36:46]W: [Step 1/1] self.name, list(config.keys())
[17:36:46]W: [Step 1/1] AssertionError: preview-ecash is not a valid build identifier. Valid identifiers are ['templates', 'builds']
[17:36:46] : [Step 1/1] ~/buildAgent/work/jailed-build
[17:36:46]W: [Step 1/1] + RESULT=1
[17:36:46]W: [Step 1/1] + popd
[17:36:46]W: [Step 1/1] + exit 1
[17:36:46]W: [Step 1/1] Process exited with code 1
[17:36:46]E: [Step 1/1] Process exited with code 1 (Step: Command Line)
[17:36:46]E: [Step 1/1] Step Command Line failed
[17:36:46]E: Ant JUnit report watcher
[17:36:46]E: [Ant JUnit report watcher] No reports found for paths:
[17:36:46]E: [Ant JUnit report watcher] +:results/test_bitcoin.xml
[17:36:46]E: [Ant JUnit report watcher] +:results/**/junit_results*.xml
[17:36:46] : Publishing internal artifacts (4s)
[17:36:51] : [Publishing internal artifacts] Publishing 1 file using [ArtifactsCachePublisher]
[17:36:51] : [Publishing internal artifacts] Publishing 1 file using [WebPublisher]
[17:36:46]W: Publishing artifacts (5s)
[17:36:46] : [Publishing artifacts] Collecting files to publish: [+:results/**/junit_results*.xml, +:bitcoin-abc/abc-ci-builds/preview-ecash/gitian-results => preview-ecash.tar.gz, +:bitcoin-abc/abc-ci-builds/preview-ecash/*.log => artifacts.tar.gz]
[17:36:46]W: [Publishing artifacts] Artifacts path 'results/**/junit_results*.xml' not found
[17:36:46]W: [Publishing artifacts] Artifacts path 'bitcoin-abc/abc-ci-builds/preview-ecash/gitian-results' not found
[17:36:46]W: [Publishing artifacts] Artifacts path 'bitcoin-abc/abc-ci-builds/preview-ecash/*.log' not found
[17:36:53] : Build finished