TokTok / website

The TokTok website
https://toktok.ltd/
GNU General Public License v3.0
10 stars 54 forks source link

Add line break before "loading pull requests". #35

Closed iphydf closed 7 years ago

iphydf commented 7 years ago

Currently it's directly following the "click on ..." paragraph.


This change is Reviewable

cebe commented 7 years ago

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


toktok/pulls.html, line 12 at r1 (raw file):

review. Click on the branch to see the pull request on GitHub.</p>

<div class="loading"><p>Loading Pull requests...</p></div>

no need to wrap the <p> in a <div>. we should put the class on the <p> instead and adjust the JS to remove that p element.


Comments from Reviewable

robinlinden commented 7 years ago
:lgtm_strong:

Reviewed 2 of 2 files at r2. Review status: all files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.


toktok/static/js/pr-table.js, line 20 at r2 (raw file):

  .then(function(response) { return response.json(); })
  .then(function(json) {
    var loadingDiv = repoSection.querySelector("p.loading");

You could leave this as just .loading and we wouldn't have to touch the js if we feel like changing the tag again.


Comments from Reviewable

cebe commented 7 years ago

Reviewed 1 of 2 files at r2. Review status: 1 of 2 files reviewed at latest revision, 2 unresolved discussions, some commit checks failed.


toktok/static/js/pr-table.js, line 20 at r2 (raw file):

Previously, robinlinden (Robin Lindén) wrote…
You could leave this as just `.loading` and we wouldn't have to touch the js if we feel like changing the tag again.

Done.


Comments from Reviewable

cebe commented 7 years ago

Reviewed 1 of 2 files at r2, 1 of 1 files at r3. Review status: all files reviewed at latest revision, all discussions resolved, some commit checks failed.


Comments from Reviewable

iphydf commented 7 years ago

Review status: all files reviewed at latest revision, 1 unresolved discussion.


toktok/pulls.html, line 12 at r1 (raw file):

Previously, cebe (Carsten Brandt) wrote…
no need to wrap the `

` in a `

`. we should put the class on the `

` instead and adjust the JS to remove that p element.

Thanks for fixing that.


Comments from Reviewable

robinlinden commented 7 years ago

Reviewed 1 of 1 files at r3. Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable