ngraham requested changes to this revision. ngraham added subscribers: chempfling, gladhorn. ngraham added a comment. This revision now requires changes to proceed.
There are a lot of good changes in here, like those keyboard navigation improvements! And I appreciate all the work that clearly went into this. However, the problem with huge patches like this is that if we like some but not all of it, you end up needing to re-work a lot of the patch. That's generally why we prefer "atomic" changes with one change per patch/commit. It makes that kind of thing way saner. And I'm afraid I don't think we can do #1. @chempfling and @gladhorn have been working hard to push on accessibility, and one thing I've learned in the past week weeks is how important focus is. Making sure that the active element both has and looks like it has focus is critical to making sure the UI is accessible for screen readers. As such, we need to keep it visually focused by default when it opens. I would be open to making the search field behave like it does in Kicker: visually focused by default, but then focus shifts to list items when they're active (but you can still type-to-search anywhere). One more note: while compatibility with 3rd-party themes is a laudable goal, it can never be a primary one, and it can't dictate design considerations. 3rd-party themes are a moving target and chasing them is futile, so "this change makes it look better with 3rd-party themes" can never be a selling point. It has to be better for Breeze first and foremost. If the change also improves the look for 3rd-party themes, all the better, but that can't be the primary consideration. So I'd like to see the font size changes go into one patch, the keyboard navigation improvements go into a second, and the search field placeholder text change in a third one. I think those are fairly non-controversial. REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D16988 To: rooty, #plasma, #vdg, romangg, ngraham, davidedmundson Cc: gladhorn, chempfling, filipf, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart
