Adding the tile section to the homepage now that core tech page is there
Details
- Reviewers
bytesofman - Group Reviewers
Restricted Project - Commits
- rABC7c9f38a1689c: [e.cash] Add tile section to homepage
preview the site and scroll down to the section on the homepage. Check everything looks good and the links work
Diff Detail
- Repository
- rABC Bitcoin ABC
- Branch
- homepage-tiles
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 24092 Build 47794: Build Diff Build 47793: arc lint + arc unit
Event Timeline
Build Bitcoin ABC Diffs / Diff Testing (preview-e.cash) passed.
Preview is available at http://54.39.19.73:41252 for the next 60 minutes.
web/e.cash/components/h2/index.js | ||
---|---|---|
11 ↗ | (On Diff #40711) | what happens if center is not specified? |
links load with title under nav bar, would be nice if they loaded with the title just below nav bar. Maybe further down or something like "centered in screen" , so that the animation is not cut off
what we have now:
would be nice:
Build Bitcoin ABC Diffs / Diff Testing (preview-e.cash) passed.
Preview is available at http://54.39.104.128:41647 for the next 60 minutes.
What do you think about mirroring the placement of the "Lean more" tile ? The site is in english, reading from left to right and it would make sense imo to have the tile on the left, and the techno tiles on the right.
This should be done in another diff though.
web/e.cash/components/homepage-tiles/index.js | ||
---|---|---|
59 ↗ | (On Diff #40711) | If you want to make the uppercase, it's better to do it with css so it is consistent across all GlitchText elements |
web/e.cash/pages/core-tech.js | ||
83 ↗ | (On Diff #40711) | dito, and the same in other places in the code that are not from this diff |
for sure, should be resolved when D14025 lands and I rebase here. Can unstage this one until then
web/e.cash/components/h2/index.js | ||
---|---|---|
11 ↗ | (On Diff #40711) | nothing, optional param, the image just wont be centered |
web/e.cash/components/homepage-tiles/index.js | ||
9 ↗ | (On Diff #40711) | oh yeah not needed, slip of habit there |
25 ↗ | (On Diff #40711) | for sure will move |
59 ↗ | (On Diff #40711) | Its not a universal thing, there are some GlitchText items that are not uppercase |
web/e.cash/pages/core-tech.js | ||
83 ↗ | (On Diff #40711) | sure, is there consensus on whether it can done in both places (ie in the html and with CSS)? on an earlier diff I was told to either do it directly in the text or with css but not both |
web/e.cash/pages/core-tech.js | ||
---|---|---|
83 ↗ | (On Diff #40711) | What do you think is the best ?
|
Build Bitcoin ABC Diffs / Diff Testing (preview-e.cash) passed.
Preview is available at http://54.39.19.73:41636 for the next 60 minutes.
- The top hand images on every page are really high for some reason: https://e.cash/wealth-redefined ; contrast that with prod:
Build Bitcoin ABC Diffs / Diff Testing (preview-e.cash) passed.
Preview is available at http://54.39.104.128:41354 for the next 60 minutes.
web/e.cash/pages/core-tech.js | ||
---|---|---|
31 ↗ | (On Diff #40865) | are all these props required? how does this component behave if some are missing? this function could use a /**
header with this kind of discussion. imo it's okay to say props are required as long as
we don't really want to be handling an edge case like "developer put a bad prop in" |
the on-hover bg color change for this tile seems pretty dark, makes it impossible to see the shadow of the hand and almost impossible to see the lower hand, both stealth and normal
Build Bitcoin ABC Diffs / Diff Testing (preview-e.cash) passed.
Preview is available at http://54.39.19.73:41064 for the next 60 minutes.
Build Bitcoin ABC Diffs / Diff Testing (preview-e.cash) passed.
Preview is available at http://54.39.19.73:41255 for the next 60 minutes.