Opened 9 years ago
Closed 9 years ago
#12409 closed enhancement (fixed)
[patch] arrow on edit relation button lists recent relations
Reported by: | kolesar | Owned by: | kolesar |
---|---|---|---|
Priority: | normal | Milestone: | 16.02 |
Component: | Core | Version: | |
Keywords: | Cc: |
Description
Added a button to relation list dialog for opening last edited relation. It is useful when you have closed the dialog, changed selection and new selection is not member of the relation.
Last relation is stored in properties of data layer. Button is enabled/disabled on layer change. Action has an empty keyboard shortcut customizable by user.
Attachments (6)
Change History (29)
by , 9 years ago
Attachment: | EditLastRelation.patch added |
---|
follow-up: 2 comment:1 by , 9 years ago
comment:2 by , 9 years ago
Replying to bastiK:
I understand the problem and where you are coming from. But my feeling is that this is too special and not the right place in the GUI. In addition it is not immediately clear what "last relation" refers to: Last selected - no; last clicked in the relation list - no. Last one opened in relation editor - bingo!
What I could imagine is a drop down button like in selection editor which lists the last couple of edited relations.
This action opens the relation that was closed last time. It works as an undo for closing relation editor. Similar to History / Recently closed windows / <first entry> Ctrl+Shift+T
in web browsers.
I did not want to complicate existing Edit button and popping up last closed seemed to be a good trade-off. I will update this patch to remember a list of recently closed relations and shortcut will pop up first entry. This way shortcut can be pressed repeatedly, similar to browser tabs.
follow-up: 4 comment:3 by , 9 years ago
Still not a good design. Josm has these small arrows (e.g. on search button) for such purposes. Don't introduce new methods for similar purposes.
comment:4 by , 9 years ago
Replying to stoecker:
Still not a good design. Josm has these small arrows (e.g. on search button) for such purposes. Don't introduce new methods for similar purposes.
I meant (but have not written explicitly) to add small arrow to open dropdown list to edit button, similar to search button. It means I will drop additional button introduced in the first version. Is it OK?
comment:6 by , 9 years ago
Replying to bastiK:
Yes, this is good.
I have rewritten feature. Renamed to RecentRelationsAction
, manages a list of recent relations. Placed a small arrow to edit relation button. Arrow opens a popup menu with recent relations. Arrow is disabled when popup list would be empty. Checks for deleted relations, undo/redo operations and layer changes.
Most recent can be opened with keyboard shortcut Shift+Escape
that is currently available. Shortcut key is displayed in list when assigned.
Opening relation editor removes entry from list, closing dialog adds to top of list. CreateMultipolygonAction also adds new relation to list, this is useful for quick editing.
by , 9 years ago
Attachment: | RecentRelationsAction.patch added |
---|
comment:7 by , 9 years ago
Owner: | changed from | to
---|---|
Summary: | [patch] edit last relation → [patch] arrow on edit relation button lists recent relations |
(I have changed owner but I do not have commit privilege yet. I ask someone from the team to review and commit.)
comment:8 by , 9 years ago
Milestone: | → 16.02 |
---|
follow-up: 11 comment:10 by , 9 years ago
thanks for the patch, but for future ones please check that:
- no checkstyle violation is added
- no public API without javadoc is added
- no invalid javadoc is added (missing params for example)
comment:11 by , 9 years ago
Replying to Don-vip:
thanks for the patch, but for future ones please check that:
- no checkstyle violation is added
- no public API without javadoc is added
- no invalid javadoc is added (missing params for example)
Sorry for my early mistakes. I'm just warming into Java development.
Are the validation tools documented in Development Guidelines? I'm afraid not. Javadoc is mentioned but way of generating/checking is not. I have extended DevelopersGuide/StyleGuide with automated checks, please verify them.
I have not found a way to run findbugs
on specific files from the command line, only from ant
that processes all files. This takes a very long time and produces a 7000+ lines .xml file. It is hard to find bugs related to my changes.
I'm still using plain text editor for editing and ant
for build because I was unable to set up Eclipse workspace correctly. Added project, build starts, finds a lot of errors like this:
Duplicate methods named spliterator with the parameters () and () are inherited from the types Collection<B> and Iterable<B>
Can you help me?
comment:12 by , 9 years ago
It's not that we expect patches to be 100% correct. Vincent's comment requests that you try to be clean, not that your always reach that goal. Most of the rules are pretty obvious, so you can follow them also without the checking tools.
If you look at the automated builds you see that we have about 249 FindBugs entries, 21 of these with high priority. 240 Coverity issues with 19 high impacts (thought Coverity hangs with the update, some of these should already be fixed). But we are clean for style and Javadoc checks.
Goal is to get rid of these issues. Ultimate solution to find new issues easier in the log is to fix all outstanding ones :-)
by , 9 years ago
Attachment: | latestrelationslist.png added |
---|
follow-up: 14 comment:13 by , 9 years ago
Resolution: | fixed |
---|---|
Status: | closed → reopened |
comment:14 by , 9 years ago
Replying to Klumbumbus:
There is an empty space before the text (position of mouse pointer). Whats the reason for this? Could we display the icon there (in this case the river icon)?
On Linux (GTK+) there was no empty space before the text. Thank you for the idea, implemented in a single line of code.
by , 9 years ago
Attachment: | IconForRecentRelations.patch added |
---|
comment:16 by , 9 years ago
Resolution: | fixed |
---|---|
Status: | closed → reopened |
Don't use hardcoded icon sizes. ImageProvider has function to get size information.
by , 9 years ago
Attachment: | IconSizeNotHardcoded.patch added |
---|
comment:17 by , 9 years ago
I have just copied code from OsmPrimitivRenderer. Attached patch patch calls ImageProvider for size.
comment:18 by , 9 years ago
If there are other places with hardcoded image sizes, they need to be fixed as well...
comment:20 by , 9 years ago
I was also working on the same topic, you were faster. Thank you for refactoring.
comment:21 by , 9 years ago
Please remember with the HIDPI the mapping will be no longer direct, but probably factor*size. I wanted to implement this in getImageSizes() after the switch. Now it probably would go into getImageSize() and getImageDimension(). But keep this future adaption in mind when changing more.
comment:22 by , 9 years ago
Resolution: | fixed |
---|---|
Status: | closed → reopened |
Modified code to hide arrow when there are no recent relations. Previously arrow was disabled that was rendered as a gray rectangle on GTK+ look&feel. When right bar was set to narrow, this small rectangle partly covered edit icon.
by , 9 years ago
Attachment: | RecentRelationsHideArrow.patch added |
---|
I understand the problem and where you are coming from. But my feeling is that this is too special and not the right place in the GUI. In addition it is not immediately clear what "last relation" refers to: Last selected - no; last clicked in the relation list - no. Last one opened in relation editor - bingo!
What I could imagine is a drop down button like in selection editor which lists the last couple of edited relations.