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

add ability to link to a particular mascot #805

Merged
merged 1 commit into from
Jul 23, 2024
Merged

add ability to link to a particular mascot #805

merged 1 commit into from
Jul 23, 2024

Conversation

mansona
Copy link
Member

@mansona mansona commented Mar 17, 2021

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:

Screenshot 2021-03-17 at 15 16 19

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

@github-actions
Copy link

Files that got Bigger 🚨:

File raw gzip
ember-website.js +42 B +15 B
ember-website.css +65 B +26 B

Files that stayed the same size 🤷‍:

File raw gzip
auto-import-fastboot.js 0 B 0 B
blurhash.js 0 B 0 B
ember-website-fastboot.js 0 B 0 B
polyfill-evergreen.js 0 B 0 B
polyfill-legacy.js 0 B 0 B
polyfill-shared.js 0 B 0 B
vendor.js 0 B 0 B
vendor.css 0 B 0 B

ijlee2
ijlee2 previously requested changes Mar 18, 2021
Copy link
Member

@ijlee2 ijlee2 left a 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:

  1. 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.

  2. When visiting the URL with a fragment identifier, the page can still scroll to the top due to logic in application route.

  3. The code change currently ignores accessibility. The focus is on top of the page, not where the specified image is.

  4. 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?

@mansona mansona marked this pull request as draft April 22, 2021 15:31
@stale
Copy link

stale bot commented Jul 13, 2021

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!

@stale stale bot added the stale label Jul 13, 2021
@stale stale bot closed this Jul 20, 2021
@ijlee2 ijlee2 deleted the mascot-ancors branch July 28, 2021 04:46
@mansona mansona restored the mascot-ancors branch June 27, 2024 14:58
@mansona mansona reopened this Jun 27, 2024
@stale stale bot removed the stale label Jun 27, 2024
Copy link

netlify bot commented Jul 23, 2024

Deploy Preview for ember-website ready!

Name Link
🔨 Latest commit 81ab6d7
🔍 Latest deploy log https://app.netlify.com/sites/ember-website/deploys/669f74cc8dde240008c18f77
😎 Deploy Preview https://deploy-preview-805--ember-website.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@mansona mansona dismissed ijlee2’s stale review July 23, 2024 09:16

The functionality is currently implemented, this just improves it

@mansona mansona marked this pull request as ready for review July 23, 2024 09:16
Copy link

2/14 Files got Bigger 🚨:

Details
File raw gzip
ember-website.js +21 B +3 B
ember-website.css +65 B +26 B

12/14 Files stayed the same size 🤷‍:

Details
File raw gzip
blurhash.js 0 B 0 B
chunk.143.js 0 B +1 B
chunk.177.js 0 B 0 B
chunk.178.js 0 B -3 B
chunk.208.js 0 B 0 B
chunk.488.js 0 B 0 B
ember-website-fastboot.js 0 B 0 B
polyfill-evergreen.js 0 B 0 B
polyfill-legacy.js 0 B 0 B
polyfill-shared.js 0 B 0 B
vendor.js 0 B 0 B
vendor.css 0 B +2 B

Created by ember-asset-size-action

Copy link
Contributor

@MinThaMie MinThaMie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Amazing :)

@mansona mansona merged commit ad17785 into main Jul 23, 2024
7 checks passed
@mansona mansona deleted the mascot-ancors branch July 23, 2024 09:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants