Adding the roadmap section to the homepage. Also adding jest and some tests to test some styling logic
Details
Details
- Reviewers
bytesofman Fabien - Group Reviewers
Restricted Project - Commits
- rABC3f083278e8e4: [e.cash] Add roadmap section
-Preview the site with @bot preview-e.cash
-Scroll down to the roadmap section and check everything looks good
-Check mobile styles
-Check both themes
-Run npm run test to see if tests pass
-Then try forcing an error by updating a roadmap item status to 'foobar'. Try npm run dev and npm run build. You should see an error
- Can also try to fix that error by adding a foobar key and value to all instances of getStatusValues
- Can also test other failures by omitting items from getStatusValues or removing a status key from the values object in getStatusValues
Diff Detail
Diff Detail
- Repository
- rABC Bitcoin ABC
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
There are a very large number of changes, so older changes are hidden. Show Older Changes
| web/e.cash/helpers/helpers.js | ||
|---|---|---|
| 21 ↗ | (On Diff #40242) | From the content of this file, helper.js is not a meaningful name. Since it's all about the roadmap status, why not call it status.js ? or roadmap.js if you plan to add more not-status stuff in it. |
| web/e.cash/helpers/helpers.js | ||
|---|---|---|
| 21 ↗ | (On Diff #40242) | Yeah, originally I thought we'd be able to use mocha to unit test this, and then just keep one file with all the helper functions. Looking at the next documentation though it will probably be more straightforward to implement jest. In this way, you would want to keep functions in each component folder that needs them, so this should be, say, components/roadmap/methods.js, then follow that convention for other components. |
| web/e.cash/__tests__/roadmap.test.js | ||
|---|---|---|
| 6–30 ↗ | (On Diff #40261) | use one describe block for these functions, which are both related to the roadmap page |
Comment Actions
Build Bitcoin ABC Diffs / Diff Testing (preview-e.cash) passed.
Preview is available at http://51.68.37.192:41686 for the next 60 minutes.
Comment Actions
Stealth mode should use different colors than nomral for "complete", "underway", and "planning" on the roadmap
Still seeing this width issue
Comment Actions
Stealth mode should use different colors than nomral for "complete", "underway", and "planning" on the roadmap
Why? Seems like a subjective design choice, and I'm kind of still figuring out what exactly the stealth theme is as we go. So far its been make everything black and white, but this may be boring, so may play with lightly using the blue and pink, or could develop some different colors.
I do think maybe some like neon green and yellow could look cool with all the black and white, so could try that here, but it may also be better to stick with a light use of the established brand colors...
Comment Actions
yeah it is subjective, i just think it looks bad if they are not changed from the old theme at all. if you really think that's the best option, go for it.
otherwise, pick whatever you think is cool. imo easier to make this decision now then later, it does have to be made at some point. it could just be some variation of grayscale.
Comment Actions
I do think maybe some like neon green and yellow could look cool with all the black and white, so could try that here, but it may also be better to stick with a light use of the established brand colors...
go for it
Comment Actions
also, if you run npm test, a folder coverage/ is created in the repo
add coverage/ to the .gitignore so this is not included in the repo
| web/e.cash/__tests__/roadmap.test.js | ||
|---|---|---|
| 15 ↗ | (On Diff #40265) | What does it return for status 'foobar' ? Is that the expected behavior ? |
| web/e.cash/__tests__/roadmap.test.js | ||
|---|---|---|
| 15 ↗ | (On Diff #40265) | For sure, will look into it |
| web/e.cash/components/roadmap/methods.js | ||
|---|---|---|
| 9 ↗ | (On Diff #40280) | if you make this error in the codebase, does this error cause npm run build to fail? Ideally the answer is yes. That way, if the page builds, we know all the helper functions are sound. I'm not sure what happens at the moment if this error is introduced by a dev using the function improperly. |
| web/e.cash/components/roadmap/methods.js | ||
|---|---|---|
| 9 ↗ | (On Diff #40280) | Yep it does. I tried changing some statuses and using wrong keys etc and it fails in npm run dev and npm run build |
Comment Actions
The build failed due to an unexpected infrastructure outage. The administrators have been notified to investigate. Sorry for the inconvenience.
Comment Actions
The build failed due to an unexpected infrastructure outage. The administrators have been notified to investigate. Sorry for the inconvenience.
Comment Actions
Build Bitcoin ABC Diffs / Diff Testing (preview-e.cash) passed.
Preview is available at http://54.39.104.128:41706 for the next 60 minutes.
Comment Actions
colors look nice
I'm still not understanding this issue
We don't want to land a diff that knowingly breaks something.
It needs to be fixed, might as well fix it here.
Comment Actions
colors look nice
I'm still not understanding this issue
We don't want to land a diff that knowingly breaks something.
It needs to be fixed, might as well fix it here.
argh my bad I thought I had it resolved. But for sure should be fixed before landing
Comment Actions
Build Bitcoin ABC Diffs / Diff Testing (preview-e.cash) passed.
Preview is available at http://51.68.37.192:41157 for the next 60 minutes.
Comment Actions
Build Bitcoin ABC Diffs / Diff Testing (preview-e.cash) passed.
Preview is available at http://51.68.37.192:41508 for the next 60 minutes.
Comment Actions
Build Bitcoin ABC Diffs / Diff Testing (preview-e.cash) passed.
Preview is available at http://51.178.130.230:41710 for the next 60 minutes.
| web/e.cash/components/roadmap/methods.js | ||
|---|---|---|
| 1 ↗ | (On Diff #40290) | methods.js is even worst as a name than helper.js. roadmap/status.js is fine. |
| 19 ↗ | (On Diff #40290) | The checks and errors make no sense. The icon should contain the status as a key, but don't list the expected keys here. There is no reason this function could not work on other status given there is an icon for it. |
| web/e.cash/components/roadmap/index.js | ||
|---|---|---|
| 89 ↗ | (On Diff #40298) | What is that ? |
| web/e.cash/components/roadmap/index.js | ||
|---|---|---|
| 89 ↗ | (On Diff #40298) | ah whoops, just a foobar key value I was testing with. forgot to remove |