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

Allow additional logo in CG spec #821

Merged
merged 2 commits into from Feb 8, 2017

Conversation

nickevansuk
Copy link
Contributor

@nickevansuk nickevansuk commented Jun 9, 2016

Allow for additional logos in the CG spec template


This change is Reviewable

@halindrome
Copy link
Contributor

Review status: 0 of 1 files reviewed at latest revision, 1 unresolved discussion, some commit checks failed.


js/w3c/templates/cgbg-headers.html, line 5 [r1] (raw file):

    <a class='logo' href='http://www.w3.org/'><img width='72' height='48' src='https://www.w3.org/StyleSheets/TR/2016/logos/W3C' alt='W3C'></a>
  </p>
  <p>

The showLogos function embeds the logos in a p element already. I don't think you need the P in here.


Comments from Reviewable

@marcoscaceres
Copy link
Member

Please be sure to add a test or two. Let me know if you need a hand or anything.

@nickevansuk
Copy link
Contributor Author

nickevansuk commented Jun 9, 2016

Have fixed <p> tag situation @halindrome, thanks. Though I should mention I just copied it from w3c/templates/headers.html (which also includes <p> tags). Should that also be corrected?

@marcoscaceres considering this feature exists on other templates, where would you advise adding a test?

@marcoscaceres
Copy link
Member

marcoscaceres commented Jun 10, 2016

@nickevansuk, looks like tests/spec/w3c/headers-spec.js would be the right place to add a test. It would just be a simple like:

  it("adds multiple logos to CG spec", function(done) {
    var ops = makeStandardOps();
    var newProps = {
       specSatus: "CG-DRAFT", 
       logos: [{}, {}]
    };
    Object.assign(ops.config, newProps);
    makeRSDoc(ops, function(doc) {
      // Find the logos
      // Please avoid using jQuery here - we are trying to stop relying on it 
    }).then(done);
  });

@marcoscaceres
Copy link
Member

@nickevansuk, in the test above, you want to check against both specStatus "CG-DRAFT", and "CG-FINAL" . You can get the logo code from the logos example in our Wiki.

@marcoscaceres marcoscaceres merged commit ea7aec9 into w3c:develop Feb 8, 2017
@marcoscaceres
Copy link
Member

I'll add tests for this locally, but the change is tiny and just reuses existing infra.

marcoscaceres added a commit that referenced this pull request Feb 8, 2017
* develop:
  v10.0.0
  Allow additional logo in CG spec (#821)
  feat: significantly improve ARIA support (#1065)
  fix(core/markdown): elements are not corrently positioned (#1071)
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.

None yet

3 participants