Display a label and username
https://www.figma.com/file/a1nkbWgbgjRlrOY9LVurTz/Comm-%2F-Desktop-app?node-id=1272%3A88750
Depends on D3320
Details
Details
- Reviewers
• benschac atul • jacek - Commits
- rCOMM18dec803fe3a: [web] Add logged in section
Render the page instead of Calendar and check if it matches the design and if current username was displayed
Diff Detail
Diff Detail
- Repository
- rCOMM Comm
- Lint
No Lint Coverage - Unit
No Test Coverage
Event Timeline
| web/settings/account-settings.css | ||
|---|---|---|
| 28–31 ↗ | (On Diff #10024) | We're settings these so that username is truncated. Setting the color is necessary here, as the color of ellipsis is taken from parent instead of the child that is truncated (we want for the ellipsis to have the same color as the username). |
| web/settings/account-settings.css | ||
|---|---|---|
| 13 | I like to avoid margin as much as possible. Could we get the same result using padding or flex-box? | |
| 23 | nit: could padding-bottom: 24px work? | |
| web/settings/account-settings.react.js | ||
| 11 | It's wild that this screen would show up in a logged out state and force us to write this conditional. | |
| 22 | doing a git grep of the project we don't normally add strings like this `{'Logged in as '} . Why do it this way here? | |
| web/settings/account-settings.css | ||
|---|---|---|
| 13 | Ok, will change that | |
| 23 | No, as the border is rendered between the padding and the margin. We can use padding top though. | |
| web/settings/account-settings.react.js | ||
| 11 | This screen shouldn't be visible for logged out users... We can replace this conditional with invariant, but I prefer avoiding them because they caused a lot of issues in the past. We can also just render null (or some message) in case of the user being logged out, which sounds like a reasonable solution here. | |
| 22 | This string ends with a space, and if you search for '} then there are a couple of usages. If this notation is confusing I can use non-breaking space (which might be a better fit here). | |
| web/settings/account-settings.react.js | ||
|---|---|---|
| 22 | that makes sense! | |