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

Capture Screen Per URL in a sitemap #74

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open

Conversation

umutphp
Copy link

@umutphp umutphp commented Feb 5, 2019

The "screenCapture" option should be set under "config.defaults" array when a sitemap is used.

The value should contain "filename" and It will be replaced with a string created from URL.

Closes #72

The screenCapture should be set under defaults array when a sitemap is used.
The value should contain "filename" and It will be raplaced with a string created from url
/home/travis/build/pa11y/pa11y-ci/bin/pa11y-ci.js
  200:5   error    Expected indentation of 1 tab but found 4 spaces  indent
  200:21  error    Strings must use singlequote                      quotes
  200:32  error    Strings must use singlequote                      quotes
  200:42  error    Strings must use singlequote                      quotes
  200:55  error    Strings must use singlequote                      quotes
  200:60  error    Strings must use singlequote                      quotes
  200:72  error    Strings must use singlequote                      quotes
  200:77  error    Strings must use singlequote                      quotes
200:5   error    Expected indentation of 1 tab but found 4 spaces  indent
Copy link
Member

@joeyciechanowicz joeyciechanowicz left a comment

Choose a reason for hiding this comment

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

Great work, thanks for contributing.
Can you add a unit-test for your change? Then this looks great

bin/pa11y-ci.js Outdated Show resolved Hide resolved
@joeyciechanowicz joeyciechanowicz added the status: work required The PR required additional work before it can be merged label Aug 19, 2019
Use default config.screenCapture value for the urls in config.urls array also
cipm can only install packages when your package.json and package-lock.json or npm-shrinkwrap.json are in sync. Please update your lock file with `npm install` before continuing.
@umutphp
Copy link
Author

umutphp commented Aug 22, 2019

Great work, thanks for contributing.
Can you add a unit-test for your change? Then this looks great

By the way, I did not add the unit-test yet.

@Mschuddings
Copy link

Can this also be used for for a list of urls?

for example:
{ "defaults": { "reporters": [ "cli", "pa11y-reporter-html" ], "screenCapture": "pa11y/filename.png" }, "urls": [ "https://www.example.be/", "https://www.example.be/nl",

In stead of specfying for each URL a screenCapture url.

@umutphp
Copy link
Author

umutphp commented Feb 14, 2023

Can this also be used for for a list of urls?

for example: { "defaults": { "reporters": [ "cli", "pa11y-reporter-html" ], "screenCapture": "pa11y/filename.png" }, "urls": [ "https://www.example.be/", "https://www.example.be/nl",

In stead of specfying for each URL a screenCapture url.

@Mschuddings I do not remember what I did for this MR :)

@kramar
Copy link

kramar commented Apr 15, 2024

Is there a chance that this PR will be merged?

@danyalaytekin
Copy link
Member

Hi @kramar, thanks for following up! Sooo reading through this conversation, it looks like the team approved the general idea, but we'll need some test coverage first. From my own point of view I'm not sure we need filenamify. Adding to future milestone for now.

@btabaska
Copy link

This is an issue for a project that I am working on. We currently use sitemap.xml and would like artifacts from every page scanned. However, the current logic of the application using a JSON config file makes it so that every screenshot is saved to the same file name when an action is run. Is there any work that can be done on this to get this PR across the finish line?

@umutphp
Copy link
Author

umutphp commented Jun 25, 2024

@btabaska 5 years ago I have created the PR but I don know why I could not finalize it. Now, I am not able to write the tests and finalize it.

@joeyciechanowicz Is it possible for you to take action on this PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: work required The PR required additional work before it can be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Variable name in screenCapture path ?
7 participants