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

Appends follow-up UI fixes #38837

Merged
merged 5 commits into from
Feb 19, 2024
Merged

Appends follow-up UI fixes #38837

merged 5 commits into from
Feb 19, 2024

Conversation

iethree
Copy link
Contributor

@iethree iethree commented Feb 15, 2024

Description

A couple little things came up during today's demo:

  1. we should auto-select the only model to append if there's only 1
    only-one

  2. there was a bug where if you did certain things in the ui (like create a new question), we would re-trigger an error modal.

Tests

  • tests have been added

@iethree iethree requested a review from a team February 15, 2024 22:41
@metabase-bot metabase-bot bot added the .Team/AdminWebapp Admin and Webapp team label Feb 15, 2024
Copy link

github-actions bot commented Feb 15, 2024

Codenotify: Notifying subscribers in CODENOTIFY files for diff 08b8dfb...e9f1400.

Notify File(s)
@ranquild frontend/src/metabase/status/components/FileUploadStatus/FileUploadStatus.tsx
frontend/src/metabase/status/components/FileUploadStatus/FileUploadStatus.unit.spec.tsx

@iethree iethree added the backport Automatically create PR on current release branch on merge label Feb 15, 2024
Copy link

replay-io bot commented Feb 15, 2024

Status Complete ↗︎
Commit e9f1400
Results
⚠️ 1 Flaky
2316 Passed

@mazameli
Copy link
Contributor

we should auto-select the only model to append if there's only 1

IMO, if there are any to append to, we should just auto-select one of them, since even dumbly selecting the first one alphabetically has a higher probability of being the one the user intended (>0% chance of being right) than the probability that no selection is the correct one (exactly 0%, haha). If it's easy to be slightly smarter (like auto selecting the one with the most recent last_edited_at date or something), so much the better.

@iethree iethree added this to the 0.49 milestone Feb 16, 2024
@iethree
Copy link
Contributor Author

iethree commented Feb 16, 2024

we should auto-select the only model to append if there's only 1

IMO, if there are any to append to, we should just auto-select one of them, since even dumbly selecting the first one alphabetically has a higher probability of being the one the user intended (>0% chance of being right) than the probability that no selection is the correct one (exactly 0%, haha). If it's easy to be slightly smarter (like auto selecting the one with the most recent last_edited_at date or something), so much the better.

ok, now this will auto-select the most recently update model (which will also be the only model if there's just one)

Comment on lines +52 to +53
useEffect(
function setDefaultTableId() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I named this function especially for @sloansparger

Copy link
Contributor

@sloansparger sloansparger left a comment

Choose a reason for hiding this comment

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

lgtm!

@iethree iethree enabled auto-merge (squash) February 19, 2024 18:16
@iethree iethree merged commit 5078594 into master Feb 19, 2024
109 checks passed
@iethree iethree deleted the appends/ui-cleanup branch February 19, 2024 18:51
github-actions bot pushed a commit that referenced this pull request Feb 19, 2024
metabase-bot bot added a commit that referenced this pull request Feb 19, 2024
Co-authored-by: Ryan Laurie <30528226+iethree@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport Automatically create PR on current release branch on merge .Team/AdminWebapp Admin and Webapp team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants