-
Notifications
You must be signed in to change notification settings - Fork 8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(cxl-ui): add new team dashboard #352
Conversation
Starting Draft PR so we are able to start testing each component early. The header is available |
size-limit report 📦
|
fbc9372
to
7024d04
Compare
Solid thank you. |
Task linked: CU-86ayd0hrw Team dashboard |
abb9bb4
to
46f20bd
Compare
f92a705
to
dab9b00
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<a href="my-account/teams/${this.teamId}/members/"> | ||
<vaadin-button class="invite-manage" theme="secondary"> | ||
Invite & manage team | ||
</vaadin-button> | ||
</a> | ||
<a> | ||
<a href="my-account/teams/${this.teamId}/settings/"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These links should be passed by attribute. We need flexibility just in case URL changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@freudFlintstone this is missing
render() { | ||
return html` | ||
<div class="container"> | ||
<header> | ||
<h1 class="title">Team progress & stats</h1> | ||
<div class="actions"> | ||
<a> | ||
<a href="roadmap/team/?team_id=${this.teamId}"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be passed by attribute
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@freudFlintstone this is missing
37b1fd5
to
c33e988
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Commit message changes:
- feat(cxl-ui): add new team dashboard header
+ feat(cxl-ui): add cxl-dashboard-team-header component
- feat(cxl-ui): add new team dashboard stats
+ feat(cxl-ui): add cxl-dashboard-team-stats component
- feat(cxl-ui): handle cta links in header and stats components
+ feat(cxl-ui): team dashboard - CTA links in header and stats components
Broken on small mobile and tablet:
Resolve conflicts |
@pawelkmpt It's not broken. You probably forgot to run yarn build-styling after changing branches. |
c33e988
to
712297c
Compare
I visually test mostly on the Storybook attached to the PR. I just tested in two ways: dev tools and Storybook mobile control and both are fixed for tablet but still broken for small mobile (missing padding). We do style things starting at 320 px width. You seem to keep forgetting it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small mobile 320px
@pawelkmpt I try to use a mobile first approach on style, as it's simpler and our standard media queries (in the _mq.scss file) all use min-width thresholds. That way I don't forget about it, but I'll check more sizes next time. The problem here was that the Fixed by forcing different box-sizing on inner container div. |
c4084fd
to
2058249
Compare
@freudFlintstone please rebase on top of |
2058249
to
201fa9a
Compare
201fa9a
to
bf74455
Compare
bf74455
to
340684f
Compare
https://app.clickup.com/t/86ayd0hrw