New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rename figure #251

Merged
merged 9 commits into from Jan 16, 2018

Conversation

Projects
None yet
4 participants
@will-moore
Copy link
Member

will-moore commented Nov 3, 2017

See https://trello.com/c/dfdmk6Pk/81-rename-figure

To test:

  • For a previously saved figure, click on the name to edit it.
  • Should be shown an input field to update name
  • When focus is lost from input (click elsewhere) or you hit Enter, the name should be updated in the browser (page title (window/tab-name) changes) and input field is replaced again by new name
  • This enables the Save button (if not previously enabled) and is added to the Undo/Redo queue.
  • When figure is saved and page refreshed, new name should be preserved and File > Open should show new figure name in list.
  • Test that unicode names and HTML characters <script>, double quotes etc are handled without errors.
  • For new unsaved files, can click where the name should be and insert new name. When you save this will be the default option for saved name.
@bramalingam

This comment has been minimized.

Copy link
Member

bramalingam commented Nov 15, 2017

Tried all the workflows and everything works as expected.

Not sure if this is a bug but,

  1. Try renaming a file , save button gets enabled. After I save the file, I did cmd+z (undo), and the name of the file went back to the old file name. But the save button did not get enabled. I would have expected it to be enabled, if the undo operation was switching the name back to the old file name.
@will-moore

This comment has been minimized.

Copy link
Member Author

will-moore commented Nov 20, 2017

@bramalingam This seems to be working for me now... Can you show me?

@bramalingam

This comment has been minimized.

Copy link
Member

bramalingam commented Nov 20, 2017

Hmmmm, seems to be working now! And in safari as well, not sure how it did not work on Friday.

@will-moore

This comment has been minimized.

Copy link
Member Author

will-moore commented Nov 20, 2017

Yeah, I seem to remember when you first reported it that it wasn't working for me, but seems fine now both locally and on web-dev-merge.

@will-moore

This comment has been minimized.

Copy link
Member Author

will-moore commented Nov 20, 2017

If this is good to merge, then it would be great to merge it since the Travis fix is needed to get all other PRs passing. @jburel

@jburel jburel added this to the 3.2.0 milestone Nov 20, 2017

@will-moore will-moore referenced this pull request Nov 21, 2017

Merged

Add labels from tags #254

@jburel

This comment has been minimized.

Copy link
Member

jburel commented Nov 23, 2017

instead of capping flake8
you should declare the expected exception in the script
That's what I had to do in this PR
ome/scripts#136

l.parent = omero.model.ImageI(i.getId(), False)
l.child = omero.model.FileAnnotationI(file_id, False)
links.append(l)
lnk = omero.model.ImageAnnotationLinkI()

This comment has been minimized.

@jburel

jburel Nov 23, 2017

Member

link might be ok :-)

@snoopycrimecop

This comment has been minimized.

Copy link
Member

snoopycrimecop commented Nov 24, 2017

Conflicting PR. Removed from build FIGURE-merge#587. See the console output for more details.
Possible conflicts:

  • PR #239 will-moore 'remove_create_original_file_from_file_obj'
    • omero_figure/views.py
  • PR #243 will-moore 'Big images'
    • omero_figure/views.py
  • PR #247 will-moore 'list files user middlename'
    • omero_figure/views.py

--conflicts Conflict resolved in build FIGURE-merge#624. See the console output for more details.

@jburel

This comment has been minimized.

Copy link
Member

jburel commented Dec 11, 2017

Discussed today with @will-moore, he will extract the flake8 related commits out of this PR

@will-moore will-moore referenced this pull request Dec 11, 2017

Merged

Flake8 fixes #256

@will-moore

This comment has been minimized.

Copy link
Member Author

will-moore commented Dec 11, 2017

Cherry-picked flake8 commits from above to #256.
I think I'll close this until that is merged, since it may otherwise cause merge conflicts.

@will-moore will-moore closed this Dec 11, 2017

@will-moore will-moore reopened this Jan 12, 2018

@will-moore will-moore force-pushed the will-moore:rename_figure branch from 127c802 to 620b6ec Jan 12, 2018

@will-moore

This comment has been minimized.

Copy link
Member Author

will-moore commented Jan 12, 2018

@jburel I just found this PR which was tested but not merged before. Good to include this in next release.

@jburel

This comment has been minimized.

Copy link
Member

jburel commented Jan 12, 2018

Let's do a final review with the next build

@will-moore will-moore referenced this pull request Jan 12, 2018

Merged

Figure 3.2 update #284

@will-moore

This comment has been minimized.

Copy link
Member Author

will-moore commented Jan 12, 2018

Added this to Help PR: openmicroscopy/ome-help#284

@jburel

This comment has been minimized.

Copy link
Member

jburel commented Jan 15, 2018

When figure is saved and page refreshed, new name should be preserved and File > Open should show new figure name in list.

The list was not refreshed after saving.
I was expecting the list to be updated without having to click the refresh button

@jburel

This comment has been minimized.

Copy link
Member

jburel commented Jan 15, 2018

Entering ```<<<testfor example for the name does not work. When I press enter only<<`` is kept

@will-moore

This comment has been minimized.

Copy link
Member Author

will-moore commented Jan 15, 2018

@jburel Pushed 2 fixes.

@jburel

This comment has been minimized.

Copy link
Member

jburel commented Jan 16, 2018

Works nicely now
Thanks.

@jburel jburel merged commit 26a1734 into ome:master Jan 16, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment