Updated in accordance with review comments. Go easy - it's the first lambda that I've submitted...
I would rather go with the "make the markers much larger but less contrast" solution than a setting for their size.
Perhaps we really just need to look at how other editors do that.
Eye-cancer like markers are no solution either, but I agree that if people turn them on, they should be able to spot them easily.
Is not that simple.
Change only the color is fine, but then the effect is almost imperceptible,
Right now from home i'm using a 25' 2560x1440 resolution with fonts in size 10. Is a regular monitor, not the most expensive.
Even changing the color a single pixel is barely visible. And yes, the fonts are.
If i go to HiDPI screen this becomes totally impossible to see ( unless you have eagle eyes ). This cases using a light background or white.
If you are using darker backgrounds, like i use Breeze Dark, a pixel even on the most bright red color still mixed up. So, no good.
Yes isListening() would make sense.
Uploaded my patch, now with unittest: https://phabricator.kde.org/D7964
Results of the investigation:
- qWaitForWindowExposed was wrong, it only masked the real problem locally
- When the label is not able to resize in a given layout and setIndent or other calls further reduce the space available, we have to react by squeezing the text. Therefore, a custom setIndent is needed for real. Concerns regarding BC can probably be countered by checking with abi-compliance-checker.
- Respecting the chrome also in minimumSizeHint is feasible and should add some polish and sensible behaviour to the default minimum widths.
- Regarding the comparison of resize handling with QLabel, it is a design decision whether we should react to changes by resizing or squeezing. After looking through the commit history and early applications, I could imagine the original authors had the following in mind (e.g. a statusbar frame of a browser displaying the link hovered over or the file name in a status dialog of a directory copying action): The label is placed in a layout, where the actual text to display is not known yet. Therefore it makes sense to give the label as much space as possible (i.e. Expanding size policy). Now the displayed text is changed quite often over the course of the lifetime of the application. Here it is reasonable to keep the size as is to avoid flickering (i.e. squeeze by default, adjustSize needs to be called manually). In summary: No changes needed, but this could be added to the documentation.
- Testing all different size policies, no errors could be found and all resizing was in accordance to the docs (the trick was to display a frame, to differentiate between window and label and not get confused). It is worth noting that for Fixed, Minimum and MinimumExpanding text will never be squeezed (by design as well as implementation).
OK, let's get the quick fix in, but I still think my alternative patch (removing all this code and applying http://www.davidfaure.fr/2017/0001-KAcceleratorManager-set-icon-text-on-actions-to-remo.patch instead) is a better solution.
It works in my tests, although the discussion in https://git.reviewboard.kde.org/r/129663/ got a little confusing.
Grouping the startsWith() and contains() entries
Ah, excellent idea.
Yes (although I would then swap document to be after text/, to have all the "groups" together, and then the "substring searches").
In any case feel free to push.
Fair enough. Your points are stronger than mine, now that I think about it.
Use startsWith() instead of contains() for greater speed and correctness, and do this for text MIME types as well
Yes, but then startsWith() would be even more correct (and slightly faster) than contains().
It seems odd to have all of these special KIO URLs that we don't actually want to use because they're rough and underdeveloped. They're rough and underdeveloped because they're hidden by default, so nobody sees them, and nobody files bugs or submits patches for them. But I do see your point.
Opps, I've corrected the bug number.
Fri, Sep 22
The linked bug looks wrong.
Yeah I also find weird to see weblinks in there. Maybe if we call it "History" would be a bit better (and it would also match the History tab in Kickoff).
I don't think this is "semi-useful". " A Recent Documents feature in the file manager and open/save dialogs is IMHO really important, especially for lesst-technical users who use features like this on other platforms expensively instead of making extensive use of folder hierarchies. "The user can add it" is a problematic response since 99.9% of users don't know this exists, and therefore don't know that this functionality is available in the first place (and if they did, they would find it challenging to add).
You could continue and add about a dozen more "semi-useful" protocol links (ftp, settings, programs, bluetooth, and much more) ;) .
I don't think that's the route for the places panel to go. Not by default that is.
If the user wants it, it can be added just like any shortcut can be added in the places thingy. By default it should stay rather clean. recentdocuments is (in my opinion) not one that should be there by default. Also, for me personally it seems rather weird as recently visited url's are also in the recent documents.... And files I've accessed on the console don't appear in it at all (understandable though).
- use qPopulationCount to count editable marks
Kate already has many options, so we try to be careful with adding even more options.