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: update suggest group titles #10568

Merged
merged 11 commits into from
Dec 18, 2024
Merged

Conversation

nickoferrall
Copy link
Contributor

@nickoferrall nickoferrall commented Dec 10, 2024

Fix #10550

Demo: https://www.loom.com/share/8f9f18206aad44cba9bf1f3b9580f4fb

To test

  • Turn on the Suggest Groups feature
  • See that after clicking Suggest Groups, the reflection group title names look good
  • Resetting the groups works well too

Base automatically changed from feat/10532/improve-group-titles to master December 10, 2024 11:45
@nickoferrall nickoferrall changed the title update suggest group titles feat: update suggest group titles Dec 11, 2024
const newGroupHasSmartTitle = reflectionGroup.title === reflectionGroup.smartTitle

if (oldGroupHasSingleReflectionCustomTitle && newGroupHasSmartTitle) {
// Edge case of dragging a single card with a custom group name on a group with smart name
Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the demo, I go through this edge case, which now works well

Copy link
Contributor

Choose a reason for hiding this comment

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

-1 I think the intention here is: you have 2 groups: group 1 has a smart title and group 2 has only one reflection and a custom title. When we move the one card over to group 1, we want to also apply the custom title to group 1, at least that's happening on master.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated. Here's a Loom demo with that edge case: https://www.loom.com/share/e669008de75447ddafc7a303c14eca33

@nickoferrall nickoferrall marked this pull request as ready for review December 11, 2024 16:25
Copy link
Contributor

@Dschoordsch Dschoordsch left a comment

Choose a reason for hiding this comment

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

Sometimes the automatic title is prefixed with Title:
image

const newGroupHasSmartTitle = reflectionGroup.title === reflectionGroup.smartTitle

if (oldGroupHasSingleReflectionCustomTitle && newGroupHasSmartTitle) {
// Edge case of dragging a single card with a custom group name on a group with smart name
Copy link
Contributor

Choose a reason for hiding this comment

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

-1 I think the intention here is: you have 2 groups: group 1 has a smart title and group 2 has only one reflection and a custom title. When we move the one card over to group 1, we want to also apply the custom title to group 1, at least that's happening on master.

@nickoferrall
Copy link
Contributor Author

I've also updated the prompt and regex to remove Title: as that's coming up frequently for you

@nickoferrall nickoferrall merged commit 81043ad into master Dec 18, 2024
6 checks passed
@nickoferrall nickoferrall deleted the feat/10550/suggest-group-titles branch December 18, 2024 14:37
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.

Suggest Groups: update group titles
2 participants