Umbrello add tooltips for UML elements
ClosedPublic

Authored by jhayes on Feb 22 2020, 9:09 PM.

Details

Reviewers
habacker
yurchor
Summary

Add tooltips to Umbrello and change the strings to i18n().

BUG: 83444

FIXED-IN: 2.31.0 (KDE release 20.04.0)

Test Plan

Insert UML objects into respective diagrams and mouseover and pause to allow tool tip to come up. After a bit it will go away or you can click on the diagram background and it will close.

Diff Detail

Repository
R139 Umbrello
Lint
Lint Skipped
Unit
Unit Tests Skipped
jhayes requested review of this revision.Feb 22 2020, 9:09 PM
jhayes created this revision.
yurchor accepted this revision.Feb 23 2020, 6:57 AM
yurchor added subscribers: ltoscano, yurchor.

Thanks for fixing the i18n() issue.

As it was mentioned by @ltoscano , it would be good for bug handling if there is a separate line (not a mention of the number) with the command for our scripts to close the bug. That's it, literally:

BUG: 83444

This revision is now accepted and ready to land.Feb 23 2020, 6:57 AM

Thanks in advance for fixing these minor issues.

umbrello/umlwidgets/componentwidget.cpp
47

Missing space after "physical".

umbrello/umlwidgets/objectnodewidget.cpp
202

Typo:

<b>Object Node<b>

should be

<b>Object Node</b>

210

Typo:

<b>Buffer Object Node<b>

should be

<b>Buffer Object Node</b>

213

Typo:

<b>Data Object Node<b>

should be

<b>Data Object Node</b>

216

Typo:

<b>Flow Object Node<b>

should be

<b>Flow Object Node</b>

219

Missing space after "creates".

yurchor requested changes to this revision.Feb 23 2020, 9:24 AM

And two more fixes.

umbrello/umlwidgets/activitywidget.cpp
83

Extra ">" after "State".

umbrello/umlwidgets/actorwidget.cpp
38

Missing space after "An actor".

This revision now requires changes to proceed.Feb 23 2020, 9:24 AM

There is also a line for specifying the version with which this extension request was added for the news and changelog (e.g https://umbrello.kde.org/changelog.php?20.04

FIXED-IN: 2.31.0 (KDE release 20.04.0)

If the patch for this check request is added to git before 3/19/20, it will actually be available with version 2.30.80, but I suspect that more time will be needed to translate the strings

See https://community.kde.org/Schedules/release_service/20.04_Release_Schedule for more details about scheduling,

Thanks for all of the comments, I will get the minor fixes wraped up today.

John

And one more typo.

umbrello/umlwidgets/artifactwidget.cpp
40

Typo: pro-cess -> process

jhayes edited the summary of this revision. (Show Details)Feb 23 2020, 1:14 PM
jhayes updated this revision to Diff 76216.Feb 23 2020, 1:23 PM

Fix xml typos

yurchor accepted this revision.Feb 23 2020, 1:26 PM

Thanks. But again, please

BUG: 83444

not

Fixes Bug: 83444

This revision is now accepted and ready to land.Feb 23 2020, 1:26 PM
jhayes edited the summary of this revision. (Show Details)Feb 23 2020, 1:28 PM

Thanks in advance for fixing these minor typos. Sorry for overlooking them before.

umbrello/umlwidgets/associationline.cpp
714

Typo: it's -> its

719

Typo: it's -> its

jhayes updated this revision to Diff 76222.Feb 23 2020, 2:46 PM

Apologize for the typos.

The last bunch of minor issues found while translating this. Thanks in advance for fixing them and for your patience.

umbrello/umlwidgets/associationline.cpp
621

Should be "Sequence Self Message"

umbrello/umlwidgets/combinedfragmentwidget.cpp
281

Missing space after "invalid."

umbrello/umlwidgets/statewidget.cpp
442

Unfinished sentence.

jhayes updated this revision to Diff 76240.Feb 23 2020, 7:47 PM

Here you go, and thanks you for your patience.

I'm doing better with git stash now.

yurchor accepted this revision.Feb 23 2020, 7:48 PM

Thanks.

habacker added inline comments.Feb 24 2020, 7:32 PM
umbrello/umlwidgets/associationline.cpp
155

unrelated change, please omit.

585

A tooltip is missing here

737–738

A tooltip is missing here

umbrello/umlwidgets/classifierwidget.cpp
1015

This change also has nothing to do with this issue. While it's good to fix empty lines or indentations, this should be done in a separate commit. This is also true for other similar places.

umbrello/umlwidgets/objectnodewidget.cpp
181

This change has nothing to do with the bug this patch is intended for, so please omit. Instead, the actual problem, namely the missing support for translations, should be fixed. (see https://bugs.kde.org/show_bug.cgi?id=418150) this patch is intended for.

335–336

same as mentioned above

350

dito

umbrello/umlwidgets/portwidget.cpp
45–48

Please remove this line, git log already indicates that it has been removed

umbrello/umlwidgets/statewidget.cpp
133–139

Does is a unrelated change without any change, please omit.

habacker added inline comments.Feb 24 2020, 7:45 PM
umbrello/umlwidgets/associationline.cpp
527

I am not happy with the extensive change in the order of the cases, because it makes it very difficult to check what exactly has changed and if something has broken.
If a change seems necessary, e.g. an alphabetical order, which makes it easier to find individual entries, then it should be done in a separate commit beforehand.

umbrello/umlwidgets/combinedfragmentwidget.cpp
240

Please order alphabetically by case.

umbrello/umlwidgets/messagewidget.cpp
869

Please sort alphabetically by case.

umbrello/umlwidgets/notewidget.cpp
146

Please sort alphabetically by case.

umbrello/umlwidgets/objectnodewidget.cpp
201

dito, sort alphabetically

umbrello/umlwidgets/signalwidget.cpp
229

Either use an if else chain or a switch statement and please order alphabetically

umbrello/umlwidgets/statewidget.cpp
425

Please sort alphabetically

jhayes marked 30 inline comments as done.Feb 25 2020, 4:50 AM

The requested changes have been made, I will run a diff and double check then submit the changes on 2/25.

Thanks for your patience and comments.

The requested changes have been made,

Please update the patch with the changes applied

then submit the changes on 2/25.

Please wait until I can inspect the updated patch and set this review in the "ready to land" state

Thanks for you work, which is appreciated

PS: phabricator is not good at managing reviews that consist of multiple patches. A better way to do this is to create a fork from the project https://invent.kde.org/kde/umbrello, push the patches into a separate branch and create a merge request.
See https://invent.kde.org/kde/umbrello/-/merge_requests/3 for an example.

I have everything configured correctly now and have completed the merge
request.