Page MenuHomePhabricator
Differential D14354

Add scroll wrapper for big tables
ClosedPublic
Actions

Authored by chad on Oct 27 2015, 8:17 PM.
Tags
None
Referenced Files
Unknown Object (File)
Mon, Dec 16, 2:34 PM
Unknown Object (File)
Sat, Dec 14, 3:16 PM
Unknown Object (File)
Sat, Dec 14, 11:25 AM
Unknown Object (File)
Sat, Dec 14, 7:24 AM
Unknown Object (File)
Wed, Dec 4, 6:07 PM
Unknown Object (File)
Wed, Dec 4, 3:25 AM
Unknown Object (File)
Sat, Nov 30, 8:17 PM
Unknown Object (File)
Thu, Nov 21, 1:41 AM
View All Files
Subscribers
Korvin

Details

Reviewers
epriestley
Maniphest Tasks
T9609: Really really wide tables in Remarkup should scroll
Commits
rPHUa1e8e2cd2042: (stable) Merge 2015 Week 44
rPHU59f5a8d2bb82: Add scroll wrapper for big tables
Summary

Adds a wrapping div to allow scrolling of overflowed tables

Test Plan

Really really really big table in Phriction, See scrollbar

Diff Detail

Repository
rPHU libphutil
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

chad updated this revision to Diff 34649.Oct 27 2015, 8:17 PM
chad retitled this revision from to Add scroll wrapper for big tables.
chad updated this object.
chad edited the test plan for this revision. (Show Details)
chad added a reviewer: epriestley.
Herald added a subscriber: Korvin. · View Herald TranscriptOct 27 2015, 8:17 PM
chad added a task: T9609: Really really wide tables in Remarkup should scroll.Oct 27 2015, 8:17 PM
epriestley requested changes to this revision.Oct 27 2015, 8:33 PM
epriestley edited edge metadata.

That test failure is probably really you? I'm guessing it's a test checking what the output of tables looks like? Or no?

This revision now requires changes to proceed.Oct 27 2015, 8:33 PM
chad added a comment.Oct 27 2015, 8:34 PM

Operator extension not available.

epriestley added a comment.Oct 27 2015, 8:35 PM

On Build 9730, it's got this:

   FAIL  PhutilRemarkupEngineTestCase::testEngine
182	Assertion failed, expected values to be equal (at PhutilRemarkupEngineTestCase.php:56): Failed to markup HTML in file 'newline-then-block.txt'.
183	Expected vs Actual Output Diff
184	--- Old Value
185	+++ New Value
186	@@ -1,8 +1,8 @@
187	 '<p>This is a paragraph.</p>
188	 
189	 <div class="remarkup-code-block" data-code-lang="txt" data-sigil="remarkup-code-block"><pre class="remarkup-code">First line of code block.
190	 Second line of code block.</pre></div>
191	 
192	-<table class="remarkup-table">
193	+<div class="remarkup-table-wrap"><table class="remarkup-table">
194	 <tr><td>Cell 1</td><td>Cell 2</td></tr>
195	-</table>'
196	+</table></div>'
197

...which is a legitimate failure (the markup emitted by tables has changed).

chad updated this revision to Diff 34651.Oct 27 2015, 8:36 PM
chad edited edge metadata.
  • add unit test
chad added a comment.Oct 27 2015, 8:37 PM

Updated the test, but unsure what's causing the test to not run.

epriestley added inline comments.Oct 27 2015, 8:41 PM
src/markup/engine/__tests__/remarkup/table.txt
1–7

This part shouldn't change -- it's the input (what you'd type in a comment area) and we don't expect users to type the <div /> part.

7–8

From the output, it looks like the actual markup that is produced is just like this:

<div ...><table ...>
...
</table></div>

...not on separate lines like it's written?

<div ...>
<table ...>
...
</table>
</div>
chad updated this revision to Diff 34652.Oct 27 2015, 8:41 PM
chad edited edge metadata.
  • add unit test
  • more tests
chad updated this revision to Diff 34653.Oct 27 2015, 8:45 PM
  • no idea
chad added a comment.Oct 27 2015, 8:45 PM
Running unit tests...
   FAIL  PhutilRemarkupEngineTestCase::testEngine
Assertion failed, expected values to be equal (at PhutilRemarkupEngineTestCase.php:56): Failed to markup HTML in file 'simple-table-with-link.txt'.
Expected vs Actual Output Diff
--- Old Value
+++ New Value
@@ -1,3 +1,3 @@
-'<table class="remarkup-table">
+'<div class="remarkup-table-wrap"><table class="remarkup-table">
 <tr><td><a href="http://example.com" class="remarkup-link" target="_blank" rel="noreferrer">name</a></td><td>[x]</td></tr>
-</table>'
+</table></div>'
chad updated this revision to Diff 34654.Oct 27 2015, 8:47 PM
  • all tests updated
chad added a comment.Oct 27 2015, 8:48 PM

ok, i'm bad at reading, didn't really it was a new test failing each fix.

epriestley accepted this revision.Oct 27 2015, 8:51 PM
epriestley edited edge metadata.
This revision is now accepted and ready to land.Oct 27 2015, 8:51 PM
Closed by commit rPHU59f5a8d2bb82: Add scroll wrapper for big tables (authored by chad, committed by chad). · Explain WhyOct 27 2015, 8:52 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathSize
src/
markup/
engine/
__tests__/
remarkup/
4 lines
4 lines
4 lines
4 lines
remarkup/
blockrule/
3 lines

Diff 34655

View Options

src/markup/engine/__tests__/remarkup/newline-then-block.txt

Loading...
View Options

src/markup/engine/__tests__/remarkup/simple-table-with-link.txt

Loading...
View Options

src/markup/engine/__tests__/remarkup/simple-table.txt

Loading...
View Options

src/markup/engine/__tests__/remarkup/table.txt

Loading...
View Options

src/markup/engine/remarkup/blockrule/PhutilRemarkupBlockRule.php

Loading...
Phabricator · Built by Phacility