Uploaded image for project: 'IGB'
  1. IGB
  2. IGBF-1153

Convert Export Bookmark file chooser to the operating system's Native File Chooser

    Details

    • Type: Bug
    • Status: Closed (View Workflow)
    • Priority: Major
    • Resolution: Done
    • Affects Version/s: None
    • Fix Version/s: None
    • Labels:
    • Story Points:
      1
    • Sprint:
      Fall 2018 1, Fall 2018 Sprint 2

      Description

      This issue is similar to IGBF-1140
      Certain places in IGB use the operating system's native file chooser and some do not. One place that does not is under the Bookmarks tab of IGB.
      IGB Bookmarks->Export Bookmarks
      We want OS native file chooser to be displayed instead of Java Swing style window.

        Attachments

          Issue Links

            Activity

            Hide
            mason Mason Meyer (Inactive) added a comment -

            I am changing the title of this story and some of the description because there is not a "Choose Local Folder" option for this feature, that only applies to stories IGBF-1140 and IGBF-1157. I am doing this to avoid confusion in the future.

            Show
            mason Mason Meyer (Inactive) added a comment - I am changing the title of this story and some of the description because there is not a "Choose Local Folder" option for this feature, that only applies to stories IGBF-1140 and IGBF-1157 . I am doing this to avoid confusion in the future.
            Hide
            akadam3 Ashwini Kadam (Inactive) added a comment -

            Fixed the issue and pushed code my fork.
            https://bitbucket.org/ashwiniK27/integrated-genome-browser/branch/IGBF-1153

            Needs testing on Linux and Mac

            Show
            akadam3 Ashwini Kadam (Inactive) added a comment - Fixed the issue and pushed code my fork. https://bitbucket.org/ashwiniK27/integrated-genome-browser/branch/IGBF-1153 Needs testing on Linux and Mac
            Hide
            sneha Sneha Ramesh Watharkar (Inactive) added a comment -

            Issue is fixed on Linux, was able to export bookmarks using my native file chooser. Uploaded screenshot.

            Few questions about the code, probably dumb questions, but is that ExportFileFilter being used by anything any more? What about the getJFileChooser method? I did a quick search and I don't think anything from ExportFileFilter is being used anywhere else in IGB, and getJFileChooser is just an implementation from GeneralUtils that I don't think is being used either (Besides from the importbookmarks function, which I think was changed over to the native file chooser in another issue), I could be wrong. If they're not being used, should they be removed? Is there a reason why they haven't been removed?

            Otherwise, there's a typo in the issue description on the second line, "Export menthod..." but that's it, no other issues.

            Show
            sneha Sneha Ramesh Watharkar (Inactive) added a comment - Issue is fixed on Linux, was able to export bookmarks using my native file chooser. Uploaded screenshot. Few questions about the code, probably dumb questions, but is that ExportFileFilter being used by anything any more? What about the getJFileChooser method? I did a quick search and I don't think anything from ExportFileFilter is being used anywhere else in IGB, and getJFileChooser is just an implementation from GeneralUtils that I don't think is being used either (Besides from the importbookmarks function, which I think was changed over to the native file chooser in another issue), I could be wrong. If they're not being used, should they be removed? Is there a reason why they haven't been removed? Otherwise, there's a typo in the issue description on the second line, "Export menthod..." but that's it, no other issues.
            Hide
            akadam3 Ashwini Kadam (Inactive) added a comment -

            Jenny suggested right about some funtions not being used anywhere else except in 'import bookmark functionality'. I have not removed that code to be on safer side, till IGBF-1152(import bookmark functionality) gets closed. Once it is merged in master, i will update the repo and submit updated code. For now it has nothing to do with export bookmark functionality and hence wanted to get it reviewed.

            Show
            akadam3 Ashwini Kadam (Inactive) added a comment - Jenny suggested right about some funtions not being used anywhere else except in 'import bookmark functionality'. I have not removed that code to be on safer side, till IGBF-1152 (import bookmark functionality) gets closed. Once it is merged in master, i will update the repo and submit updated code. For now it has nothing to do with export bookmark functionality and hence wanted to get it reviewed.
            Hide
            akadam3 Ashwini Kadam (Inactive) added a comment - - edited

            I found following problems while testing the fix on macOS which are similar to IGBF-1184:
            1. On MAcOS bookmarks get saved with name 'bookmark1..html' or 'bookmark1..txt'. Extra '.' is appended before extension.
            2. If user saves it with full name, 'bookmark.html', error dialog is shown saying 'You cannot save this document with extension “.html” at the end of the name. The required extension is “.”. '

            This seems like MacOS problem as the fix is working fine with windows and linux.

            Moving to the 'To-Do' lane for further investigation.

            Show
            akadam3 Ashwini Kadam (Inactive) added a comment - - edited I found following problems while testing the fix on macOS which are similar to IGBF-1184 : 1. On MAcOS bookmarks get saved with name 'bookmark1..html' or 'bookmark1..txt'. Extra '.' is appended before extension. 2. If user saves it with full name, 'bookmark.html', error dialog is shown saying 'You cannot save this document with extension “.html” at the end of the name. The required extension is “.”. ' This seems like MacOS problem as the fix is working fine with windows and linux. Moving to the 'To-Do' lane for further investigation.
            Hide
            ieclabau Ivory Blakley (Inactive) added a comment -

            When this is resolved, be sure to update the documentation here:
            https://wiki.transvar.org/display/ITD/FileChooser

            Show
            ieclabau Ivory Blakley (Inactive) added a comment - When this is resolved, be sure to update the documentation here: https://wiki.transvar.org/display/ITD/FileChooser
            Hide
            sneha Sneha Ramesh Watharkar (Inactive) added a comment -

            Work is in the below branch:
            https://bitbucket.org/swathark/integrated-genome-browser/branch/IGBF-1153#diff

            Needs first level review. Thanks!

            Show
            sneha Sneha Ramesh Watharkar (Inactive) added a comment - Work is in the below branch: https://bitbucket.org/swathark/integrated-genome-browser/branch/IGBF-1153#diff Needs first level review. Thanks!
            Hide
            ieclabau Ivory Blakley (Inactive) added a comment - - edited

            It seems odd to export bookmarks in either html or txt, but we can only import bookmarks in html format. I assume the txt format is supposed to be a more readily human readable format . Should we indicate that to the user during export?
            _____________________

            Git review:
            All in one commit, good.
            For the commit message, use the imperative voice whenever possible (like you telling IGB how to change). "Convert export bookmarks file chooser to native file chooser"
            _____________________

            Code review:
            I don't see any problems here.
            Do remember to include spaces around '+' to make code easier to read (lines 363, 370)
            _____________________

            Functional review:
            The Bookmarks export window does use the native file chooser.

            However...
            If I try to save my file outfile.html I get the following message:
            You cannot save this document with extension “.html” at the end of the name. The required extension is “.”.
            I think we need to add a check to see if the name the user entered already has a valid file extension. This message should only appear if an invalid extension is used. ...it looks like line 371 is supposed to do that. The error message is reached sometime before we do that check.

            If this cannot be readily fixed, I think it is different enough that it would be ok to make a new issue to fix this error message.
            But it would nice to just fix it now if we can.

            _____________________

            I am sending this back to to-do and reassigning it to Sneha to fix the error message.

            Show
            ieclabau Ivory Blakley (Inactive) added a comment - - edited It seems odd to export bookmarks in either html or txt, but we can only import bookmarks in html format. I assume the txt format is supposed to be a more readily human readable format . Should we indicate that to the user during export? _____________________ Git review: All in one commit, good. For the commit message, use the imperative voice whenever possible (like you telling IGB how to change). "Convert export bookmarks file chooser to native file chooser" _____________________ Code review: I don't see any problems here. Do remember to include spaces around '+' to make code easier to read (lines 363, 370) _____________________ Functional review: The Bookmarks export window does use the native file chooser. However... If I try to save my file outfile.html I get the following message: You cannot save this document with extension “.html” at the end of the name. The required extension is “.”. I think we need to add a check to see if the name the user entered already has a valid file extension. This message should only appear if an invalid extension is used. ...it looks like line 371 is supposed to do that. The error message is reached sometime before we do that check. If this cannot be readily fixed, I think it is different enough that it would be ok to make a new issue to fix this error message. But it would nice to just fix it now if we can. _____________________ I am sending this back to to-do and reassigning it to Sneha to fix the error message.
            Hide
            sneha Sneha Ramesh Watharkar (Inactive) added a comment -

            Ivory Blakley Okay, So the above mentioned issue doesn't appear in Windows or Linux. Its seen only in MAC.
            When I went through older comments on this issue, I noticed that the fix for this issue was already in place and I adopted the same from her branch. Now when I went over all the comments I got to know that the issue was never merged because of the same issue.
            It was open for investigation.

            Show
            sneha Sneha Ramesh Watharkar (Inactive) added a comment - Ivory Blakley Okay, So the above mentioned issue doesn't appear in Windows or Linux. Its seen only in MAC. When I went through older comments on this issue, I noticed that the fix for this issue was already in place and I adopted the same from her branch. Now when I went over all the comments I got to know that the issue was never merged because of the same issue. It was open for investigation.
            Hide
            ieclabau Ivory Blakley (Inactive) added a comment -

            Okay, as a long as we have an issue to describe the MAC-specific file extension problem, this can move on to a pull request.

            Show
            ieclabau Ivory Blakley (Inactive) added a comment - Okay, as a long as we have an issue to describe the MAC-specific file extension problem, this can move on to a pull request.
            Hide
            sneha Sneha Ramesh Watharkar (Inactive) added a comment -

            After discussing with the reviewer Ivory Blakley, came to a conclusion that this issue can proceed. Created a new issue to investigate the issue specific to Mac.
            New issue is https://jira.transvar.org/browse/IGBF-1389 .

            Pushing the current issue for reviewing purpose.

            Show
            sneha Sneha Ramesh Watharkar (Inactive) added a comment - After discussing with the reviewer Ivory Blakley , came to a conclusion that this issue can proceed. Created a new issue to investigate the issue specific to Mac. New issue is https://jira.transvar.org/browse/IGBF-1389 . Pushing the current issue for reviewing purpose.
            Hide
            ann.loraine Ann Loraine added a comment -

            Merged to master.

            Show
            ann.loraine Ann Loraine added a comment - Merged to master.
            Hide
            mason Mason Meyer (Inactive) added a comment -

            After testing this story I can confirm that the Export Bookmark file chooser has been changed to the operating system's native file chooser. This has been tested on Mac and Windows and is functioning as expected. All Bookmark related features are functioning as expected and there seem to be no side effects resulting from this change. Since this issue is resolved it will now be closed.

            Show
            mason Mason Meyer (Inactive) added a comment - After testing this story I can confirm that the Export Bookmark file chooser has been changed to the operating system's native file chooser. This has been tested on Mac and Windows and is functioning as expected. All Bookmark related features are functioning as expected and there seem to be no side effects resulting from this change. Since this issue is resolved it will now be closed.

              People

              • Assignee:
                sneha Sneha Ramesh Watharkar (Inactive)
                Reporter:
                akadam3 Ashwini Kadam (Inactive)
              • Votes:
                0 Vote for this issue
                Watchers:
                5 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: