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

FEATURE: Improve user sorting behavior #5159

Open
wants to merge 4 commits into
base: 8.4
Choose a base branch
from

Conversation

crydotsnake
Copy link
Member

@crydotsnake crydotsnake commented Jun 22, 2024

Upgrade instructions

None

Review instructions

I added this feature for 9.0 but since we want to release a Neos 8.4 i cherry-picked the commit so we have this small feature already in Neos 8.4!

For more details have a look at the original PR: #4443

Checklist

  • Code follows the PSR-2 coding style
  • Tests have been created, run and adjusted as needed
  • The PR is created against the lowest maintained branch
  • Reviewer - PR Title is brief but complete and starts with FEATURE|TASK|BUGFIX
  • Reviewer - The first section explains the change briefly for change-logs
  • Reviewer - Breaking Changes are marked with !!! and have upgrade-instructions

Copy link
Member

@kitsunet kitsunet left a comment

Choose a reason for hiding this comment

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

Right, why not... But is this really all that is needed?

Copy link
Member

Choose a reason for hiding this comment

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

wait a sec, this doesnt look particular light anymore ;) it seems it was not minified?

Copy link
Member

Choose a reason for hiding this comment

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

Huh, interesting

Copy link
Member Author

Choose a reason for hiding this comment

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

I compiled everything again after the rebase 🤔.

Copy link
Member

@kitsunet kitsunet left a comment

Choose a reason for hiding this comment

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

Realized just now, did you maybe not do yarn build:production ? That seems to work fine for me.

@crydotsnake
Copy link
Member Author

Realized just now, did you maybe not do yarn build:production ? That seems to work fine for me.

Mh. I thought i exactly did that. I recompiled the files with yarn build:production

@crydotsnake crydotsnake requested review from mhsdesign and kitsunet July 2, 2024 12:38
@crydotsnake
Copy link
Member Author

Why are the PHP Tests failing?

@crydotsnake crydotsnake force-pushed the feature/improve-user-sorting branch from 4a46ffe to 6f80e14 Compare August 19, 2024 13:18
@kitsunet
Copy link
Member

I think the php errors are not your fault, but tahter a problem of 8.4 and behat...

@crydotsnake crydotsnake force-pushed the feature/improve-user-sorting branch from 6f80e14 to 1616e8d Compare September 1, 2024 16:09
@crydotsnake
Copy link
Member Author

Ready for review!

But PHP tests are still failing after a rebase..

@bwaidelich
Copy link
Member

@kitsunet I'm not sure why you blocked this, I assume the CSS/JS diff was too big. Is that solved now?

@bwaidelich bwaidelich removed their request for review October 1, 2024 10:30
Copy link
Member

@kitsunet kitsunet left a comment

Choose a reason for hiding this comment

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

yep, fine fine!

mhsdesign and others added 3 commits October 5, 2024 13:45
@crydotsnake crydotsnake force-pushed the feature/improve-user-sorting branch from 1616e8d to 1cfddd5 Compare October 5, 2024 11:46
@crydotsnake crydotsnake enabled auto-merge October 5, 2024 11:48
@crydotsnake crydotsnake force-pushed the feature/improve-user-sorting branch from 0914b45 to 453ac47 Compare October 5, 2024 11:50
@crydotsnake crydotsnake disabled auto-merge October 5, 2024 11:50
@crydotsnake crydotsnake requested a review from kitsunet October 5, 2024 11:53
@crydotsnake
Copy link
Member Author

@mhsdesign Please also have a look at it :) I had to rebase again.

@crydotsnake
Copy link
Member Author

@mhsdesign Ping :)

@crydotsnake
Copy link
Member Author

Hello @kitsunet @mhsdesign

Would be awesome if you could take a look again!

I still have no idea why the tests are failing. Maybe a second rebase could help? 🤞🏽

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.

4 participants