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

Aggregation picker changes for metrics #42053

Merged
merged 13 commits into from
May 2, 2024
Merged

Conversation

ranquild
Copy link
Contributor

@ranquild ranquild commented Apr 30, 2024

Closes #41935
Figma https://www.figma.com/file/17Z8b1uEdcrtpP8rVHaPAS/Metrics-v2?type=design&node-id=1697-3003&mode=design&t=rwgYnuNJy2ynVE8G-0

Changes:

  • "Common metrics" are moved to top and there is no title
  • The first section is always expanded

Please note that if you remove the metric aggregation clause in the notebook editor for a question that uses a metric we start to add Count by default in the summarize sidebar which looks strange/wrong. This is a separate issue .

Screenshot 2024-05-02 at 13 20 05

Copy link

replay-io bot commented Apr 30, 2024

Status Complete ↗︎
Commit f822c62
Results
1 Failed
⚠️ 9 Flaky
2423 Passed

@ranquild ranquild requested a review from a team May 2, 2024 17:03
Copy link

github-actions bot commented May 2, 2024

Codenotify: Notifying subscribers in CODENOTIFY files for diff 02e38e4...f822c62.

Notify File(s)
@kdoh frontend/src/metabase/ui/utils/colors.ts

@@ -206,7 +206,7 @@ export const AccordionListCell = ({
</div>
);
} else if (type === "header-hidden") {
content = <div className={CS.my1} />;
content = <div className={cx({ [CS.my1]: itemIndex > 0 })} />;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removes margin from the first empty header item

@@ -95,25 +94,24 @@ export function AggregationPicker({
const database = metadata.database(databaseId);
const canUseExpressions = database?.hasFeature("expression-aggregations");

if (operators.length > 0) {
if (metrics.length > 0) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

reorders sections

@ranquild ranquild requested review from uladzimirdev and removed request for a team and uladzimirdev May 2, 2024 18:11
@ranquild ranquild changed the title Aggregation picker changes for metrics [WIP] Aggregation picker changes for metrics May 2, 2024
@ranquild ranquild merged commit dfbf23e into metrics-v2 May 2, 2024
108 of 109 checks passed
@ranquild ranquild deleted the 41935-aggregation-picker branch May 2, 2024 18:35
Copy link
Contributor

@uladzimirdev uladzimirdev left a comment

Choose a reason for hiding this comment

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

👍

@ranquild ranquild changed the title [WIP] Aggregation picker changes for metrics Aggregation picker changes for metrics May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants