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 test sheet #87

Merged
merged 8 commits into from
Nov 9, 2015
Merged

Add test sheet #87

merged 8 commits into from
Nov 9, 2015

Conversation

m1
Copy link
Contributor

@m1 m1 commented Oct 30, 2015

NB. I made it so the instagram pictures are in /ref just incase instagram remove the pictures from the cssgram account or the person developing the filter has no internet access

</div>

{# cssgram #}
<div>
Copy link
Owner

Choose a reason for hiding this comment

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

Thanks for doing this!

Format for using CSSgram is as follows:

<figure class="<class">
  <img src="..." alt="...">
</figure>

The class goes on the figure (this is important because <img> elements can't have pseudo elements and I noticed some of the styling was absent because of this.

@m1
Copy link
Contributor Author

m1 commented Oct 31, 2015

Ah I did wonder why some looked a bit off.

@una
Copy link
Owner

una commented Oct 31, 2015

This is an awesome addition to the project. A couple of things:

  • I'd like to keep the source/ folder clean -- only having the product (aka the .scss or .css code in there) for when people use bower or other package managers to consume this
  • the twig files can go within site/ and there can simply be a test.html in there
  • On that note, it would be good to refactor index.html to use a templating engine since this adds one anyway (this is more me thinking out loud than asking it to be a part of this PR)
  • I'd like to .gitignore any test files until necessary for local testing
  • Making some comments on the markup 😄

You rock, by the way! 🎸

@@ -0,0 +1,80 @@
<!DOCTYPE html>
Copy link
Owner

Choose a reason for hiding this comment

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

maybe this should be in source/ and be test.twig

@m1
Copy link
Contributor Author

m1 commented Oct 31, 2015

All fair points.

I'd like to keep the source/ folder clean -- only having the product (aka the .scss or .css code in there) for when people use bower or other package managers to consume this

Should source/scss/demo-site.scss be moved then?

On that note, it would be good to refactor index.html to use a templating engine since this adds one anyway (this is more me thinking out loud than asking it to be a part of this PR)

I can do this in another pr, just want to make this pr for the test sheet only rn.

I'd like to .gitignore any test files until necessary for local testing

What do you mean by this?

Making some comments on the markup

Haha yeah, it was very quickly hacked up 😆

@una
Copy link
Owner

una commented Oct 31, 2015

Excellent point that demo-site.scss would ideally only live in site/css and would import the library within it (going to submit a PR for this)

@una
Copy link
Owner

una commented Oct 31, 2015

Check out #89 -- this should also make it easier to have a test page

@una
Copy link
Owner

una commented Nov 7, 2015

I submitted a PR on your repo @m1 in which I move the test page as a part of the site

@m1
Copy link
Contributor Author

m1 commented Nov 7, 2015

Don't merge this yet, just making a few other changes

@una
Copy link
Owner

una commented Nov 7, 2015

No problem -- I don't have Internet right now but I plan to refactor the site to use twig in another PR later today so don't worry about thay

@m1
Copy link
Contributor Author

m1 commented Nov 9, 2015

Okay that should be everything I think?

una added a commit that referenced this pull request Nov 9, 2015
@una una merged commit c09d988 into una:master Nov 9, 2015
@una
Copy link
Owner

una commented Nov 9, 2015

Nice work Miles 👍

@m1 m1 deleted the test-suite branch November 19, 2015 17:37
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.

2 participants