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

feat: new style for "package" headers #856

Closed
wants to merge 1 commit into from

Conversation

runspired
Copy link
Contributor

No description provided.

if (typeName != 'namespace') {
console.warn(
e1,
'fetching by class or module failed, retrying as namespace'
);
return this.store.find('namespace', param).catch((e2) => {
return this.store.findRecord('namespace', param).catch((e2) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this change should be separated out and landed irregardless, this code is deprecated and will break in 5.0 @jaredgalanis @jenweber

@jenweber jenweber temporarily deployed to ember-api-docs-review-template April 17, 2023 15:47 Inactive
@jenweber
Copy link
Contributor

We reviewed this at the Learning Team meeting today and like the improvements to visual hierarchy on the package pages.

We spotted 2 changes needed, then this can be merged:

  • Can we make this something other than an h1? We noticed that there are multiple h1s on some pages. The h1 is what will be picked up by SEO, as opposed to the page's actual content focus, and having multiple h1s causes accessibility issues. This was a preexisting issue, but with increased use of the package attribute and the changes to visuals that move us away from typical heading typography, it's a good time to fix it.
  • The brown text over gray does not pass color contrast standards when we checked it on https://webaim.org/resources/contrastchecker/ Oddly enough, the browser developer tools didn't catch this, maybe because some styles come from ::after.

I also noticed that the mobile look could use some iteration, but that's not a blocker. It would be a good issue for a new contributor.

Let us know if you want any help making these adjustments!

Lastly, out of scope for this PR, but maybe helpful: when writing new docs, if ember data contributors use more specific h1s like "Overview of Something" vs "Overview", the topic will show up more easily in search engine results.

@jenweber
Copy link
Contributor

P.S. Thank you for your work on this!

@MinThaMie
Copy link
Contributor

Found this PR during the core-learning meeting, since we are currently working on the #710 we'll make sure this comes along in some shape or form :)

Copy link

netlify bot commented Dec 8, 2023

Deploy Preview for ember-api-docs ready!

Name Link
🔨 Latest commit a1a2e3a
🔍 Latest deploy log https://app.netlify.com/sites/ember-api-docs/deploys/6572f1a9bd857a0007c76ff8
😎 Deploy Preview https://deploy-preview-856--ember-api-docs.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.

@MinThaMie
Copy link
Contributor

Will close this PR in favour of the new design, took a screenshot and will add that to the projectboard

@MinThaMie MinThaMie closed this Jul 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants