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

Display non-standard params on summary page #469

Merged

Conversation

camillobruni
Copy link
Contributor

@camillobruni camillobruni commented Jan 6, 2025

Make sure we better highlight non default params since they affect the default score.

  • Simplify defaultParams creation code a bit
  • Rebuild news-next since params.mjs changed
  • Display non-default params table in the summary view
Screenshot 2025-01-06 at 17 35 08

<!DOCTYPE html><html lang="en"><head><meta charSet="utf-8"/><meta name="viewport" content="width=device-width"/><meta name="next-head-count" content="2"/><link rel="preload" href="./_next/static/css/a0dca1379a01e5cf.css" as="style"/><link rel="stylesheet" href="./_next/static/css/a0dca1379a01e5cf.css" data-n-g=""/><noscript data-n-css=""></noscript><script defer="" nomodule="" src="./_next/static/chunks/polyfills-c67a75d1b6f99dc8.js"></script><script src="./_next/static/chunks/webpack-e50e9853db18b759.js" defer=""></script><script src="./_next/static/chunks/framework-2c79e2a64abdb08b.js" defer=""></script><script src="./_next/static/chunks/main-2ba37e62325cc71b.js" defer=""></script><script src="./_next/static/chunks/pages/_app-4fe6038a10d156ec.js" defer=""></script><script src="./_next/static/chunks/pages/_error-54de1933a164a1ff.js" defer=""></script><script src="./_next/static/slmX88Sy3MO0SLK8cVHnz/_buildManifest.js" defer=""></script><script src="./_next/static/slmX88Sy3MO0SLK8cVHnz/_ssgManifest.js" defer=""></script></head><body><div id="__next"></div><div id="settings-container"></div><div id="notifications-container"></div><script id="__NEXT_DATA__" type="application/json">{"props":{"pageProps":{"statusCode":404}},"page":"/_error","query":{},"buildId":"slmX88Sy3MO0SLK8cVHnz","assetPrefix":".","nextExport":true,"isFallback":false,"gip":true,"scriptLoader":[]}</script></body></html>
Copy link
Member

Choose a reason for hiding this comment

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

Why are these getting re-generated?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had to run npm install; npm run build in the news-next workload since shared/params.mjs has changed and it's included as a package in that workload.

Copy link
Member

Choose a reason for hiding this comment

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

Oh, that's unfortunate.

resources/main.mjs Outdated Show resolved Hide resolved
resources/main.mjs Outdated Show resolved Hide resolved
Copy link
Member

@rniwa rniwa left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@bgrins bgrins requested a review from julienw January 7, 2025 18:53
Copy link
Contributor

@julienw julienw left a comment

Choose a reason for hiding this comment

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

thanks, this looks good to me. Just a few simple style changes would be great, tell me what you think

resources/main.mjs Outdated Show resolved Hide resolved
resources/main.css Show resolved Hide resolved
@camillobruni camillobruni merged commit d6b5ffe into WebKit:main Jan 9, 2025
4 checks passed
@camillobruni camillobruni deleted the 2025-01-06_non_standard_params branch January 9, 2025 16:08
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