johnpicozzi → credited starshaped → .
nicxvan → credited starshaped → .
nicxvan → credited starshaped → .
nicxvan → credited starshaped → .
nicxvan → credited starshaped → .
volkswagenchick → credited starshaped → .
volkswagenchick → credited starshaped → .
volkswagenchick → credited starshaped → .
volkswagenchick → credited starshaped → .
volkswagenchick → credited starshaped → .
volkswagenchick → credited starshaped → .
starshaped → made their first commit to this issue’s fork.
volkswagenchick → credited starshaped → .
volkswagenchick → credited starshaped → .
volkswagenchick → credited starshaped → .
lauriii → credited starshaped → .
volkswagenchick → credited starshaped → .
Updated the MR to address @plopsec's comment.
Is there anything to address from #5? I'm trying to determine what the next steps are from there.
starshaped → made their first commit to this issue’s fork.
Looks good to me, tested in Firefox, Chrome, and Safari. The JS looks good to me too, always nice to shave off a few ms!
starshaped → made their first commit to this issue’s fork.
Met with @ckrina today and discussed #19. I moved the title to be back in `toolbar-block.css` with the additions that were added.
@finnsky Thanks for the review and for pointing out that the resets were resetting more than just the sidebar. I updated it to follow what you posted in the MR so take a look again. I checked it in Bootstrap, Umami, and Olivero and the resets are now staying in the admin toolbar.
Pushed some changes to rename the selector and cleaned up the specificity of the selector after renaming. Let me know if this is the right way to approach the selector, @mherchel / @ckrina!
starshaped → made their first commit to this issue’s fork.
starshaped → made their first commit to this issue’s fork.
volkswagenchick → credited starshaped → .
nicxvan → credited starshaped → .
johnpicozzi → credited starshaped → .
laura.gates → credited starshaped → .
ckrina → credited starshaped → .
ckrina → credited starshaped → .
ckrina → credited starshaped → .
@Santosh_Verma Yep, this was on purpose because the styles would not display correctly in RTL mode when using logical properties. Since using logical properties here caused a regression in the layout, I used non-logical properties instead.
Also, unassigning myself to this issue as I haven't had time to work on it--anyone, feel free to pick this one up!
@smustgrave, I actually wasn't quite done with my refactoring and still need to fix a few issues, so I'm moving this back to needs work. Thanks @andy-blum for fixing my mess, I'll work off that new MR!
ckrina → credited starshaped → .
ckrina → credited starshaped → .
volkswagenchick → credited starshaped → .
Readded the RTL styles as they only apply on an RTL layout, and added before and after screenshots.
Code has been cleaned up and tests are passing. Ready for review!
Yeah, that was a mistake on my end, I didn't realize I had changed the status! I'll set it to needs review once I get my work done.
I'm going to be working on this tonight and tomorrow to further clean up the CSS and fix the MR failures.
volkswagenchick → credited starshaped → .
I am so happy to see this issue being opened, because this is certainly an issue I've run into with clients over the years. So yes, I'm a fan of this being discussed :)
I like #8's suggestion of displaying the context by which the term is being used, so users can correctly pick the result they want. In my experience, users don't typically use the ID to determine the correct result, so providing a better context will help so much more.
I also like #10's suggestion of making autocomplete results themeable. I do think being able to provide *some* sort of context in some way is important, and adding date, teasers, etc to the result would help this as well.
Either way, something else besides IDs should be used to help users choose the correct result, and the above suggestions would definitely solve that problem.