Skip to content
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

Merged
merged 8 commits into from
Nov 15, 2023
Merged

Conversation

freudFlintstone
Copy link

@freudFlintstone
Copy link
Author

freudFlintstone commented Oct 27, 2023

Starting Draft PR so we are able to start testing each component early. The header is available

@github-actions
Copy link

github-actions bot commented Oct 27, 2023

size-limit report 📦

Path Size
packages/cxl-ui/pkg/dist-web/cxl-ui.js 66.81 KB (+2.56% 🔺)
packages/cxl-ui/pkg/dist-web/cxl-ui-jwplayer.js 11.87 KB (0%)
packages/cxl-ui/pkg/dist-web/cxl-ui-playbooks.js 27.77 KB (+0.03% 🔺)
packages/cxl-ui/pkg/dist-web/vendor.js 135.58 KB (0%)
packages/cxl-ui/pkg/dist-web/cxl-ui-jwplayer.js, packages/cxl-ui/pkg/dist-web/cxl-ui-playbooks.js, packages/cxl-ui/pkg/dist-web/cxl-ui.js, packages/cxl-ui/pkg/dist-web/manifest.js, packages/cxl-ui/pkg/dist-web/unresolved.js, packages/cxl-ui/pkg/dist-web/vendor.js 243.18 KB (+0.7% 🔺)

@freudFlintstone freudFlintstone force-pushed the raphael/feat/dashboard/team branch from fbc9372 to 7024d04 Compare October 28, 2023 02:33
@heshfekry
Copy link

Solid thank you.

@freudFlintstone
Copy link
Author

Task linked: CU-86ayd0hrw Team dashboard

@freudFlintstone freudFlintstone marked this pull request as ready for review October 31, 2023 14:27
@freudFlintstone freudFlintstone force-pushed the raphael/feat/dashboard/team branch from abb9bb4 to 46f20bd Compare October 31, 2023 14:28
@freudFlintstone freudFlintstone force-pushed the raphael/feat/dashboard/team branch from f92a705 to dab9b00 Compare November 2, 2023 20:15
Copy link

@pawelkmpt pawelkmpt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small mobile (320px):

  • missing side padding,
  • me need more space between stats and progress bar.
Screenshot 2023-11-02 at 21 11 35

Comment on lines 35 to 40
<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/">

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.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@freudFlintstone this is missing

packages/cxl-ui/src/components/cxl-dashboard-team-stats.js Outdated Show resolved Hide resolved
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}">

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@freudFlintstone this is missing

@freudFlintstone freudFlintstone force-pushed the raphael/feat/dashboard/team branch 2 times, most recently from 37b1fd5 to c33e988 Compare November 7, 2023 13:33
Copy link

@pawelkmpt pawelkmpt left a 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:

Screenshot 2023-11-08 at 13 55 28

Screenshot 2023-11-08 at 13 55 04

@pawelkmpt
Copy link

Resolve conflicts

@freudFlintstone
Copy link
Author

@pawelkmpt It's not broken. You probably forgot to run yarn build-styling after changing branches.

@freudFlintstone freudFlintstone force-pushed the raphael/feat/dashboard/team branch from c33e988 to 712297c Compare November 8, 2023 14:48
@pawelkmpt
Copy link

pawelkmpt commented Nov 9, 2023

@pawelkmpt It's not broken. You probably forgot to run yarn build-styling after changing branches.

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.

Screenshot 2023-11-09 at 11 24 37
Screenshot 2023-11-09 at 11 24 51

Copy link

@pawelkmpt pawelkmpt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small mobile 320px

@freudFlintstone
Copy link
Author

freudFlintstone commented Nov 9, 2023

@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 cxl-stats component is not properly responsive, and it pushes the boundaries of the container on the smallest screen.

Fixed by forcing different box-sizing on inner container div.

@freudFlintstone freudFlintstone force-pushed the raphael/feat/dashboard/team branch 2 times, most recently from c4084fd to 2058249 Compare November 14, 2023 11:58
@pawelkmpt
Copy link

@freudFlintstone please rebase on top of master

@freudFlintstone freudFlintstone force-pushed the raphael/feat/dashboard/team branch from 2058249 to 201fa9a Compare November 14, 2023 19:42
@pawelkmpt pawelkmpt force-pushed the raphael/feat/dashboard/team branch from 201fa9a to bf74455 Compare November 15, 2023 09:52
@pawelkmpt pawelkmpt force-pushed the raphael/feat/dashboard/team branch from bf74455 to 340684f Compare November 15, 2023 09:52
@pawelkmpt pawelkmpt merged commit a7519de into master Nov 15, 2023
4 checks passed
@pawelkmpt pawelkmpt deleted the raphael/feat/dashboard/team branch January 9, 2024 12:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants