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% 🔺)

@heshfekry
Copy link

Solid thank you.

@freudFlintstone
Copy link
Author

Task linked: CU-86ayd0hrw Team dashboard

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.

@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

@pawelkmpt pawelkmpt merged commit a7519de into master Nov 15, 2023
5 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