[Marble-devel] Review Request 125922: Fix crosshairs overlay PopupItem when reading the text

Torsten Rahn tackat at kde.org
Mon Nov 2 17:54:56 UTC 2015


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125922/#review87885
-----------------------------------------------------------



src/lib/marble/PopupItem.h (line 174)
<https://git.reviewboard.kde.org/r/125922/#comment60297>

    In Qt there are certain naming conventions that are consistently applied to the API:
    
    Signals are always phrased in the past/present perfect tense and refer to what has just happened: 
    
    fooChanged();
    
    In this case it would be e.g.
    
    popupItemVisibleChanged(bool)



src/lib/marble/layers/PopupLayer.cpp (line 194)
<https://git.reviewboard.kde.org/r/125922/#comment60299>

    if we taint the PopupItem API with cross hair stuff can we keep it to a minimum and maybe have 
    setCrosshairsVisible(bool) instead?



src/lib/marble/layers/PopupLayer.cpp (line 207)
<https://git.reviewboard.kde.org/r/125922/#comment60298>

    By doing this you override the choice that the user might have made in the menu to not show the crosshairs at all ...
    
    Before you hide the crosshairs you need to determine whether the user actually had the crosshairs enabled and store that information somewhere so that the original situation gets properly restored.


- Torsten Rahn


On Nov. 2, 2015, 5:28 nachm., Imran Tatriev wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125922/
> -----------------------------------------------------------
> 
> (Updated Nov. 2, 2015, 5:28 nachm.)
> 
> 
> Review request for Marble.
> 
> 
> Repository: marble
> 
> 
> Description
> -------
> 
> Fix crosshairs overlay PopupItem when reading the text.
> 
> Crosshairs overlay web-popup window from wikipedia and "interrupts" the reading. I've decided to automatically hide it when web-popup is open and automatically return to previous state when popup is closed.
> 
> You can see before/after screenshots down below.
> 
> 
> Diffs
> -----
> 
>   src/lib/marble/PopupItem.h 56fd7c0d11fb304adcaa546696a64d96ac0e007c 
>   src/lib/marble/layers/PopupLayer.h 1ed58bad3de9681150ae88fe65005be7c6a68f83 
>   src/lib/marble/layers/PopupLayer.cpp 8d8af29ab499f197321bd1feb836b5b84d2060ac 
> 
> Diff: https://git.reviewboard.kde.org/r/125922/diff/
> 
> 
> Testing
> -------
> 
> 
> File Attachments
> ----------------
> 
> How it looked before
>   https://git.reviewboard.kde.org/media/uploaded/files/2015/11/02/3fdaf3ee-441e-421c-9706-35819e371813__before.png
> How it looks right now
>   https://git.reviewboard.kde.org/media/uploaded/files/2015/11/02/9b53bb43-8165-46ea-9824-49b91f64dec6__after.png
> 
> 
> Thanks,
> 
> Imran Tatriev
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/marble-devel/attachments/20151102/4f54081c/attachment.html>


More information about the Marble-devel mailing list