twilio-labs / open-pixel-art Goto Github PK
View Code? Open in Web Editor NEWA collaborative pixel art project to teach people how to contribute to open-source
Home Page: https://open-pixel-art.com
License: MIT License
A collaborative pixel art project to teach people how to contribute to open-source
Home Page: https://open-pixel-art.com
License: MIT License
Update the Readme.md for correct grammar and punctuations.
actions:
comment:
message: 'This PR touches more than one file and has to be [revied] manually. @dkundel'
label:
add:
- 'needs-review'
It would be cool to show the username of the person who contributed the pixel when you hover over it. Right now there's not client-side JavaScript running on the page. I won't have time to work on this myself but it would be cool to see this.
The page currently does not have any client-side JavaScript and even with this should stay as lightweight as possible. Please do not add a framework for this as this should be possible to be solved with vanilla JavaScript.
The solution can be either a small popup above the canvas or a field below the canvas that gets populated with the right name when you hover/tap on a pixel. That way it would also work on mobile.
We want to have as many people as possible contribute a pixel to the canvas on open-pixel-art.com and teach people how to contribute to open source along the way.
In order to contribute a pixel, you'll have to contribute an entry to the _data/pixels.json
file. You can pick your favorite color and your preferred coordinates on the canvas.
You can find the exact instructions in the Contributing Guide.
If you are new to contributing to open-source, check out our mission inside TwilioQuest that will guide you through every step of contributing to this project.
The Download TwilioQuest a tag in the About the Project section is missing the href, target and rel attributes.
The Readme.md file has missing punctuations and incorrect grammar.
Is it possible to add a test to check if a new colorized pixel isn't already colorized by someone else ?
It's difficult to locate a contributed pixel since the number of pixels are growing. We need pixel search by specifying username or coordinates. Also highlight the matched pixel.
Have links to external sites open in a new window so it doesn't fully navigate you off the current page.
My screen is not so big, so when I try to hover on the top side of canvas, I can't see who contribute to it, as I need to scroll down to see them, but it would change the name by then. Or, I can see them, but I need to zoom out my browser to do that, and there's no way I would do such thing. So, what if instead of put it below the canvas, we make them as a tooltip, that would always follow mouse position.
I'm interesting on implementing this one, in fact I'm currently try doing it.
During times of high commit volume like Hacktoberfest, or when the stars align against you, someone else may be working on updating the same pixel as you are. With a generic recommended commit message, you need to dig into every open PR to view the file changes in order to check what pixels were changed in the open PRs that are ahead of yours.
The contributing guidelines give specific instructions to add your commit with the message feat(pixels): add my new pixel.
, following the Conventional Commits Standard. By adding your claimed pixel to the commit message (and, therefore, your automatic PR title), one can easily do a visual scan of the PRs waiting to be merged to make sure they aren't about to try to change a pixel that will soon be merged and cause a conflict. For example, feat(pixels): add my new pixel (8, 27).
This solution would be loosely implemented by updating that section of the docs.
Somewhat related to #340
Test is currently a placeholder:
describe('evaluatePixelChanges', () => {
test('placeholder', () => {
expect(true).toBeTruthy();
});
});
When hovering over pixels and seeing "That pixel was contributed by: ${user}"
the text is always red. Can the text be updated to match the color of the pixel? There could be visual accessibility concerns with this approach. It could be set up to colorize the username to match the contribution only if the contrast passes a certain threshold using an approach like: https://24ways.org/2010/calculating-color-contrast/
In CONTRIBUTING.md, there is a statement that unclaimed pixels have a username reference of <UNCLAIMED>
. When I was participating and looking through the _data/pixels.json
file, I couldn't find a single line that said this, despite there being plenty of unclaimed pixels when viewing the actual pixel art. I would suggest either rewriting that section, or else updating _data/pixels.json
to actually include these unclaimed lines
There exists the potential to improve the UX of this website by adding the simple ability to visit another person's GitHub profile by simply clicking on that person's pixel. I would like to work on this idea if it is available for execution.
When a pixel isn't contributed by someone it says "That pixel was contributed by:" without a username.
Could the text be ommitted or updated to say "contribute to this pixel" or something that effect and link to https://open-pixel-art.com/#contributing
I'd be interested in implementing either solution if different functionality is desired.
Hiya,
As we already have some translations, for instance Deutsch and Spanish, I though to add the translation for Brazilian Portuguese.
I have everything done and all set. Just wanna create an awareness for this one.
Right now when you hover a taken pixel it shows the contributor username on the contributor box. But when you move to an unclaimed pixel the last username is still on the box.
I think the box should show the contributor only when it has one.
Right now there is nothing in place to prevent pixels "overwriting" each other.
If this is deliberate then please close this issue.
Ideas to address this:
Just recently I found out about ISSUE and PR templates.
This site can generate one with a very interesting structure:
https://www.talater.com/open-source-templates/#/
Maybe we can tailor something along those lines to help out the contribution process when it comes to feature requests and issues/bug reports.
@dkundel Is it possible to change _site/index.html i was trying to update menu so its within container so it aligned with rest of page . but doesn't seem to allow me to?
I added a section to CONTRIBUTIONS.md which explains how to ensure that your forked branch is up to date and can be merged successfully.
Right now Danger is failing with an exception for empty PRs like this one: #14
Instead it should just fail letting people know that the PR is empty. This has to be fixed in the dangerfile.js
.
The error is the following:
TypeError: Cannot read property 'modified_files' of undefined
at handleMultipleFileChanges (dangerfile.js:19:18)
at run (dangerfile.js:145:13)
at Object.<anonymous> (dangerfile.js:157:1)
at Module._compile (module.js:652:30)
at Object.requireFromString [as default] (/Users/dkundel/dev/pixel-project-dev/node_modules/require-from-string/index.js:28:4)
at Object.<anonymous> (/Users/dkundel/dev/pixel-project-dev/node_modules/danger/distribution/runner/runners/inline.js:144:63)
at step (/Users/dkundel/dev/pixel-project-dev/node_modules/danger/distribution/runner/runners/inline.js:32:23)
at Object.next (/Users/dkundel/dev/pixel-project-dev/node_modules/danger/distribution/runner/runners/inline.js:13:53)
at /Users/dkundel/dev/pixel-project-dev/node_modules/danger/distribution/runner/runners/inline.js:7:71
at new Promise (<anonymous>)
In your local environment run:
npx danger pr https://github.com/twilio-labs/open-pixel-art/pull/14
With the fix applied this should output a fail('This PR is empty and needs a manual review')
.
Test is currently a placeholder:
describe('allPatchesAreForTheSamePixel', () => {
test('placeholder', () => {
expect(true).toBeTruthy();
});
});
Right now the validation fails in Danger if your username is differently cased than what you placed in the pixels.json
file. Instead when we do the check in the dangerfile.js
we should first lower case both the username on GitHub and the one in the pixels.json
and then compare the two.
When you try to hover the top left pixel, some pixel doesn't show up the tooltip. I try to search where the problem is, and found out that .screenreader-only
element cover up some of the pixels. This can solve easily by adding z-index: -1;
to .screenreader-only
styles, but is it fine to do that? It wouldn't changes the accessibility, right?
z-index: -1;
z-index: -1;
Application has links to github and twilio at the footer.But links to facebook, twitter and linkedIn are missing.
There was a spelling mistake in readme.txt. It was written 'pix' instead of 'pick'.
The contributing guide refers to format:json
which does not exist. It should instead be format:sort-pixels
.
By default the placeholders have the format <UNCLAIMED>
. When people claim those pixels they might be inclined to set their username as <dkundel>
for example. It would be nice to have a unit test in __tests__/data.js
that checks for this. Otherwise it's only being caught in CI.
This was found by @abdulajet
The "every pixel should be in the limits of the image" test in data.tests.js
currently only validates that x and y are not greater than the width and height of the canvas, but should also validate that x and y are 0 or greater.
I did my PR and saw that it did not pass the review. However, looking at other recent PRs, I realized that many are being rejected. What appears is that I modified many lines, but only added my pixel
With 40 by 40 pixel grid, when a new pixel is inserted, the newly added pixel is very difficult to locate.
It requires a new tag At
after That pixel was contributed by @username
so that it should look like That pixel was contributed by @username at (x,y)
where x and y is the pixel value.
Currently user does not know where in the grid is (0,0) to start the count and find the newly added pixel.
I think It will be a good addition to the participants' experience if on pixel hover if it's unassigned, the box would display that pixel's coordinates. That way it would be easier for people to pick the available pixel spots they like. I'd like to work on this feature.
#455 Should address this, but it was detected as if I'm claiming multiple pixels, but I'm just moving other people's claims to different positions.
The contributing docs are really excellent and beginner-friendly.
One issue that contributors might run into during Hacktoberfest is that new commits by other contributors could make their fork go out of sync with the upstream repository before they have finished their own pull request.
Adding a link to GitHub's instructions on syncing a fork, or a paragraph with similar instructions, could be useful.
The website has no party mode. Pixels don't ever move. This needs to be addressed.
The contributing guide has some grammar errors, such as redundancy and wrong punctuation.
Hi!
The snippet below is matching true when some false "hex code" is inputed.
I've tried with #####AAA0f111a and the tests passed.
test('every pixel should have a hex code color if present', async () => {
const pixels = await loadJson('pixels.json');
for (const pixel of pixels.data) {
const hasColor = typeof pixel.color !== 'undefined';
if (hasColor) {
expect(pixel.color).toMatch(/#[0-9a-f]{3,6}/i);
}
}
});
Can I get this one? I already know to fix it.
Test is currently a placeholder:
describe('isValidPixelUpdate', () => {
test('placeholder', () => {
expect(true).toBeTruthy();
});
});
'Commiting' should be changed to 'Committing'
While following the CONTRIBUTIONS.md instructions, when you run an npm install
from the main directory, you get the following warning:
✔ Configured custom merge driver
added 1214 packages from 742 contributors and audited 884553 packages in 15.08s
found 1 high severity vulnerability
run `npm audit fix` to fix them, or `npm audit` for details
After running an npm audit
, you get the following message:
=== npm audit security report ===
┌──────────────────────────────────────────────────────────────────────────────┐
│ Manual Review │
│ Some vulnerabilities require your attention to resolve │
│ │
│ Visit https://go.npm.me/audit-guide for additional guidance │
└──────────────────────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ High │ Machine-In-The-Middle │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package │ https-proxy-agent │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in │ >=3.0.0 │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ danger [dev] │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path │ danger > https-proxy-agent │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info │ https://npmjs.com/advisories/1184 │
└───────────────┴──────────────────────────────────────────────────────────────┘
found 1 high severity vulnerability in 884553 scanned packages
1 vulnerability requires manual review. See the full report for details.
And, last, after running npm audit fix
, this is what I get:
up to date in 4.926s
fixed 0 of 1 vulnerability in 884553 scanned packages
1 vulnerability required manual review and could not be updated
Expected behavior: the main npm install of this package should not include any high vulnerability installations.
As someone that uses docker a lot I think it would be nice to add one or more Dockerfiles for a local docker
setup.
That way, we would minimize the chance of conflicting versions of node and give another alternative for those that want to contribute for something more than one pixel. I am open for ideas about what kind of setup are we looking at, but if this is an approved feature, I'll get to work on this ASAP.
Add IDE specific folders in .gitignore
Danger failed on the PR #34 even though it was a valid PR stating that the PR was not all on the same line.
There seems to be some logic issue that requires some investigation and debugging.
Danger: ⅹ Failing the build, there is 1 fail.
## Failures
Please make sure all of your changes are on the same line and that you are only modifying one row.
npx danger pr https://github.com/twilio-labs/open-pixel-art/pull/34
You should see the error message outlined above. Instead it should pass as a valid submission.
A declarative, efficient, and flexible JavaScript library for building user interfaces.
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google ❤️ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.