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 /logos/ route new sections and anchor links #1009

Merged

Conversation

samridhivig
Copy link

This MR works on the Copy/Content Updates to /logos/ page #1007 issue tasks:

  1. Each section has an ID that can be used in the anchor link in order to navigate to a specific section
  2. Added id to each Mascot ListItem li i.e mascot.id
  3. Added the necessary sections
  4. Added links to each instance of the Standard Art Set
  5. Added the links to the mascot items specified in Standard Art Set

@netlify
Copy link

netlify bot commented Apr 14, 2023

Deploy Preview for ember-website ready!

Name Link
🔨 Latest commit d5b8a6e
🔍 Latest deploy log https://app.netlify.com/sites/ember-website/deploys/669e8fff2e458e0008561f23
😎 Deploy Preview https://deploy-preview-1009--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.

Copy link
Member

@mansona mansona left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution 🎉 can you just remove the yarn.lock and we can get this merged? we're just using npm on this project so we don't have a yarn.lock file here 👍

@wifelette
Copy link
Member

It looks like the anchors on the Mascots aren't working. If you follow the new links on the Brand page, the URL is updating, but the mascots page jumps back to the top. Just me?

This looks otherwise amazing, thank you @samridhivig so much for doing the work, and sorry I've been offline and haven't said that sooner!

@MinThaMie
Copy link
Contributor

MinThaMie commented Apr 24, 2023

Same here, in the preview and running it locally ;) I'll check at the learning meeting if somebody there knows why.

@mansona
Copy link
Member

mansona commented Apr 24, 2023

So this came up a long time ago when I tried to implement the same thing. The issue is that we need to enable an experimental feature on Fastboot that will allow this to work as expected 🙈 (It's EXPERIMENTAL_RENDER_MODE_SERIALIZE for reference)

We discussed this in the Learning team today and we think it's probably best to update this PR to not include the links to the specific Tomsters/Zoes and we can fix the linking problem in a different PR. Essentially we're not 100% comfortable enabling the experimental feature without testing it and we didn't want to block the other important bits of this PR.

What do you think?

@mansona mansona force-pushed the content-updates-to-logos-route branch from dc5c442 to d5b8a6e Compare July 22, 2024 16:59
@mansona
Copy link
Member

mansona commented Jul 22, 2024

It turns out we figured out how to make the links work with #1120 so I've rebased this and if the links are working on the demo site I'll get it merged 👍

@MinThaMie MinThaMie merged commit 30d68ed into ember-learn:main Jul 23, 2024
6 checks passed
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.

4 participants