Skip to content
This repository has been archived by the owner on Dec 15, 2022. It is now read-only.

open relative links on click in preview, fixes #85 #442

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

tlnagy
Copy link

@tlnagy tlnagy commented Oct 18, 2016

Based on sleuthing by @rscottfree and @brandoncurtis

@tlnagy
Copy link
Author

tlnagy commented Oct 19, 2016

@50Wliu @as-cii are the CI failures anything to worry about? They look unrelated to this PR

@winstliu
Copy link
Contributor

Yes, they are related to this PR. The changes you committed do not follow the style guidelines.

@tlnagy
Copy link
Author

tlnagy commented Oct 19, 2016

Thanks for the pointers, hopefully my code will get past the linter this time.

@tlnagy
Copy link
Author

tlnagy commented Oct 19, 2016

Travis is passing now, not sure what is happening with the spec failures on windows.

@winstliu
Copy link
Contributor

Windows is 💥 💣 💀. Don't worry about it.

@winstliu
Copy link
Contributor

Also, this will most likely need some specs.

@tlnagy
Copy link
Author

tlnagy commented Oct 19, 2016

Also, how should we handle the case when the relative link points to something that doesn't exist? Currently, my code just open an empty markdown preview and that's it.

I'm pretty new to the javascript/atom ecosystem. I'll see what I can do with writing some specs.

@tlnagy
Copy link
Author

tlnagy commented Oct 19, 2016

describe "when a relative path is clicked", ->
    it "opens the link in a new pane", ->
      preview.destroy()
      preview.element.remove()

      filePath = atom.project.getDirectories()[0].resolve('subdir/relpath1.md')
      preview = new MarkdownPreviewView({filePath})
      jasmine.attachToDOM(preview.element)

      waitsForPromise ->
        preview.renderMarkdown()

      runs ->
        atom.commands.dispatch preview.element, 'click'

      runs ->
        console.log(preview.element)

I writing some tests, but do you have suggestions on how to simulate the clicking of the url? I want to check that it loads the other file.

@winstliu
Copy link
Contributor

It looks like you're already simulating the click with atom.commands.dispatch preview.element, 'click'. Then you should be able to check if 1) a new pane has been spawned, and 2) if the new pane contains an item with the filepath that you clicked on.

As for dealing with nonexistent code paths, I would suggest either:

@tlnagy
Copy link
Author

tlnagy commented Oct 19, 2016

I am simulating a click, but I don't think I'm actually clicking on the link. Is that possible? I would like to see preview change to the linked file

@brandoncurtis
Copy link

When clicking a link to a nonexistent destination, the notification could give the option to create a new file at the destination.

@brandoncurtis
Copy link

brandoncurtis commented Oct 25, 2016

The event.stopPropagation() line appears to be preventing me from opening web links after applying this patch. Commenting out this line fixes the problem:

      'click': (event) =>
        #event.stopPropagation()
        @followLink(event)

@brandoncurtis
Copy link

Currently this will also open links to the local document's anchors as empty files. Anchor links should jump to the anchor in the preview. It looks like this could be implemented using @syncPreview, but my hacking around has yet to get it working:

  followLink: (event) ->
    return false if @loading
    if event.target.tagName is 'A' and event.target.protocol is 'file:'
      activeFile = @getPath()
      activeFileDir = path.dirname(activeFile)
      clickedFile = event.target.getAttribute('href')
      if clickedFile.indexOf("#") == 0
        ### DO SOMETHING WITH @syncPreview?
      else
        clickedPath = path.join(activeFileDir, clickedFile)
        atom.workspace.open clickedPath, {split: 'left'}

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants