-
-
Notifications
You must be signed in to change notification settings - Fork 187
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
add ability to link to a particular mascot #805
Conversation
Files that got Bigger 🚨:
Files that stayed the same size 🤷:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, @mansona. At the moment, I'm not sure what benefits jumping to a particular image in the Mascots page brings. Could you explain how you need to use this feature?
Fragment identifiers present a few problems:
-
We can't change image file names to improve maintenance (my understanding is that the file name results in the ID), without additional code for redirects.
-
When visiting the URL with a fragment identifier, the page can still scroll to the top due to logic in
application
route. -
The code change currently ignores accessibility. The focus is on top of the page, not where the specified image is.
-
The code change doesn't indicate to a user that fragment identifiers are available and which identifier to use. They would need to know how the app is built, by looking at the repo or inspecting the DOM.
I would like to suggest that you open a feature request issue so that we can gauge how much the community needs to jump to an image in Mascots page. Would this be okay with you?
This pull request has been automatically marked stale. If this pull request is something that still needs work, please add a comment and it will remain open, otherwise it will close in 7 days. You are welcome to open a new pull request if you miss the window. Thanks! |
✅ Deploy Preview for ember-website ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
The functionality is currently implemented, this just improves it
2/14 Files got Bigger 🚨: Details
12/14 Files stayed the same size 🤷: Details
Created by ember-asset-size-action |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Amazing :)
I found myself wanting to link to the Empress mascot recently so I added the ability to link to a particular mascot using an ID.
Once this PR is merged you will be able to go to https://emberjs.com/mascots/#empress and it should scroll to the right part of the page and highlight the "target" mascot like this:
Let me know if you have any questions :)
Edit: here is a link with the preview app to allow you to test if it's working https://deploy-preview-805--ember-website.netlify.app/mascots/#empress
Second Edit: this functionality was added in #1009 and this PR now just adds the highlight to the linked Mascot