- User Since
- Mar 7 2016, 5:11 PM (158 w, 5 d)
Tue, Mar 12
Mon, Mar 11
Fri, Mar 1
@ngraham Thanks for the guidelines
Thu, Feb 28
I never landed a diff before because I just got my KDE developer account.
Few general questions:
- Is it enough that one reviewer accept the revision, or all the listed ones should accept it? (or depends on the project?)
- Can I directly land it once the condition above is satisfied, or is it always better to ask before landing it?
Mon, Feb 25
Sun, Feb 24
Feb 20 2019
- Revert lessons ids
- Substitude non ASCII characters
Feb 10 2019
Note that the uuid of the course is the same while the uuids of the lessons have changed. Not sure what should be changed and what not.
Jan 18 2019
Maybe @andreask knows what are the most suitable icons to use.
Jan 17 2019
Yes, that is what I meant.
Probably the collapsed by default (current situation) is ok if there is the button.
I use the side review panel only when I need to go through all the reviews one by one, so it is annoying having to expand all the elements by hand. Maybe a expand/collapse all solution would be a better solution. (The long answer is in T8553 ).
Elaborating a bit on why I proposed some changes based on my main usage pattern.
Jan 16 2019
I have not even started working on this task, so your help is more than welcome. I agree that may be useful discussing the points before writing too much code. In particular, I think that the architecture of the proxy model should be done properly given that it is the culprit of some points in the list. For me it was not straightforward how to implement it properly.
Jan 2 2019
Ok, I think it should be ok now.
- Add lesson with all accented letters, better word balance for rare words
I would like to add few improvements to this lesson (to fix few problems I am noticing only now), so I ask you to wait before working on this review.
I completely agree with you, having two different lessons can be beneficial.
- Fix lesson title and description
Oct 14 2018
Oct 13 2018
Oct 9 2018
Oct 8 2018
Oct 4 2018
Current situation on a 1680px wide screen:
Should compile now. Typewriter tool is not included yet. I add few more actions but the color picker works only for text annotation tools for now.
- Check if chosen color is valid before storing it
- Notify PageViewAnnotator when the color has changed
- Add missing annotation tools actions
- Add XML annotation tools and connect corresponding actions
Sep 18 2018
Another UI problem to solve is how to deal with some specific annotation tools "advanced" configurations:
- Some tools have an icon (popup note, stamp)
- Straight line tool has Line extensions
- Inline note has font, font size, alignment
Yours should go first. I think the toolbar thing is going to take some time both because I'll work on it intermittently and because it may require many changes under the hood. Let's see if I can manage to get it ready for KDE Applications 18.12.
Sep 17 2018
@tobiasdeiminger ..and yes I would like to coordinate so we don't waste effort.
@tobiasdeiminger mentioned in D15580 that long pressing to select different geometrical shapes (polygon, ellipse) is not very intuitive. I agree on this. I implemented it that way to be consistent with the other tool buttons of okular, for example the selection button (text selection, table selection, ...). In particular the buttons are based on ToolAction which derives from KSelectAction. I would change the default button mode of ToolAction to QToolButton::MenuButtonPopup, but I believe we can open a new bug for this and do it for all Okular buttons.
I have added the revision D15580 where I implemented a first (very rough) version of the toolbar, so we can start discussing the best way to implement it.
To test this code temporary move .config/okularpartrc or the annotation tools may not match the ones in the buttons.
Sep 12 2018
Just tested again with my solution on top of sander one and the bottom border disappears, but not the upper one. Strange, I didn't remember this effect when I tested it months ago.
I arrive a little late, but testing Okular today I noticed a minor bug with your solution. If you slowly scroll the page by dragging it with the browse tool the upper/lower dark border are not drawn sometimes when the word is at the border of the page. See bug 398553. With the "solution" I proposed instead this was not happening IIRC.
Sep 10 2018
Much better now. Thanks!
Sep 8 2018
I was thinking that a way to facilitate a newcomer getting into a project could be to find him a code buddy, another newcomer working on the same project.
May 16 2018
The link to the Mentoring page is literally the last word of the Getting involved page. I did read the getting involved page (maybe with not too much attention), but I did not reach the mentoring page and did not figured out I could contact a developer directly. Maybe this link can be highlight better somehow?
May 15 2018
May 13 2018
@andreask Thanks for the quick reply. I'll soon make a list of the icons required and let you know.
May 12 2018
Here are my 2 cents.
May 11 2018
Ok, thanks for testing again. I re-tried your patch with the latest master (just to check that there weren't modifications that solved the problem) and I still can reproduce the error. With my modification it goes away. So let's wait for aacid to review the diff.
This is a first mockup of the annotation toolbar.
May 10 2018
Have you tried also what I suggested to reproduce it? i.e. Position an highlighted word that match the search at the top boundary of the view so that it is half visible and then scroll to show it completely. (Just to confirm this is not reproducible)
May 9 2018
@sander Have you had the time to try the fix I suggested?
- Focus text edit of annotation when double-clicking on annotation icon
Apr 24 2018
I'll have a look at it. I think that focusing it makes sense also to locate which popup note is associated with the annotation clicked (especially if they are not overlapped).
Apr 22 2018
Indeed I noticed that analyzing the code. I'll try to rewrite the proxies to solve these problems.
Apr 19 2018
- Propagate dataChanged in Author and Page proxy models
Apr 18 2018
- Propagate the event
Apr 7 2018
You're not really pushing a "branch" with arc, you can think of it more as a diff describing a range of commits.
This review was added by mistake and can be deleted.
I have created a new revision D12013 by mistake. That can be deleted.
- Remove extra space
- Remove extra space
Add suffix "with comment" instead of asterisk for more clarity
- Set object name in AnnotWindow (for autotest)
- Add basic autotest for AnnotWindow
- Test AnnotWindow raised when clicking on its annotation
- Test click on AnnotWindow raises it
- Remove trailing spaces
Apr 6 2018
What is the best way to implement this? Given the call to i18n I guess I cannot just append a suffix to ret.
Should I do for each annotation type something like:
bool hasComment = !ann->contents().isEmpty(); ret = hasComment ? i18n( "Highlight with Comment" ) : i18n( "Highlight Note" );
Thanks to your suggestions I made some progresses.
It was a pleasure!
Mar 31 2018
I have few questions regarding the internal structure of okular and how to write autotests:
Mar 30 2018
Ok. I figured it out myself.
Mar 29 2018
I've progressed a bit on this, but now I get the following error: