Skip to content
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

Direct links to image lead to routing error #105

Closed
j-maas opened this issue May 14, 2020 · 4 comments
Closed

Direct links to image lead to routing error #105

j-maas opened this issue May 14, 2020 · 4 comments
Projects

Comments

@j-maas
Copy link
Contributor

j-maas commented May 14, 2020

I've tried adding links to image's source, so that people can open them full screen if they want to. Unfortunately, when the image is clicked, it reports Page not found. Valid routes: , blog, blog/draft, blog/hello

If I open the link in a new tab, it loads fine, because the error is with elm-pages interception of a link click. I'm not sure exactly what is wrong, but there is definitely the assumptions that such links point to pages, and not directly to images like in this case.

Reproduction

  1. In the elm-pages-starter, replace the last definition in src/Page/Article.elm with the following.
    articleImageView : ImagePath Pages.PathKey -> Element msg
    articleImageView articleImage =
        Element.link []
            { url = ImagePath.toString articleImage
            , label =
                Element.image [ Element.width Element.fill ]
                    { src = ImagePath.toString articleImage
                    , description = "Article cover photo"
                    }
            }
  2. Run npm run start (or npm run serve), go to http://localhost:3000/blog/hello and click on the image of the book. It should show the error.
  3. Now select the url bar and press enter to trigger a new request. It should load the image.
  4. You can play around with pressing the browser's back and forward button and clicking on the image.
@tennety
Copy link

tennety commented May 14, 2020

I thought this was already fixed before 🤔 Just for reference, in case it helps with any regressions: #17

@dillonkearns dillonkearns added this to To prioritize in Roadmap Jun 17, 2020
@kyasu1
Copy link
Sponsor

kyasu1 commented Jun 28, 2020

I have also encountered with this problem and here are my thoughts.

The easiest work around is just open the image in another tab by using newTabLink, this works as expected. Because all links are once routed to the internal update function as normal Elm app, it is possible to support the behavior as you mentioned if we add another condition to hard reload the page at the following code if the target link is images.

LinkClicked urlRequest ->
case urlRequest of
Browser.Internal url ->
let
navigatingToSamePage =
(url.path == model.url.path) && (url /= model.url)
in
if navigatingToSamePage then
-- this is a workaround for an issue with anchor fragment navigation
-- see https://github.com/elm/browser/issues/39
( model, Browser.Navigation.load (Url.toString url) )
else
( model, Browser.Navigation.pushUrl model.key (Url.toString url) )
Browser.External href ->
( model, Browser.Navigation.load href )

But I think the Elm way is creating a page dedicated to display images, so we can keep the app state and navigate pages without loosing user experiences.

@j-maas
Copy link
Contributor Author

j-maas commented Aug 28, 2021

In the new version, opening images on the dev server works as expected.

@j-maas j-maas closed this as completed Aug 28, 2021
@dillonkearns
Copy link
Owner

Thank you for trying it out on the latest version, I was just wondering about this 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Roadmap
  
To prioritize
Development

No branches or pull requests

4 participants