1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-12-03 04:02:43 +01:00
Commit graph

203 commits

Author SHA1 Message Date
epriestley
7574be5372 Remove opacity effects for left-side / right-side diff text selection
Summary:
These effects feel like they're possibly overkill, since other CSS rules make the selection reticle behave correctly and the implementation is relatively intuitive.

Or not, either way.

Test Plan: Selected text on either side of a 2-up diff, no more opacity effects.

Reviewers: amckinley

Reviewed By: amckinley

Differential Revision: https://secure.phabricator.com/D20264
2019-03-12 12:40:01 -07:00
epriestley
a3ebaac0f0 Tweak the visual style of the ">>" / "<<" depth change indicators slightly
Summary:
Ref T13249.

  - When a line has only increased in indent depth, don't red-fill highlight the left side of the diff. Since reading a diff //mostly// involves focusing on the right side, indent depth changes are generally visible enough without this extra hint. The extra hint can become distracting in cases where there is a large block of indent depth changes.
  - Move the markers slightly to the left, to align them with the gutter.
  - Make them slightly opaque so they're a little less prominent.

Test Plan: See screenshots.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13249

Differential Revision: https://secure.phabricator.com/D20251
2019-03-07 11:46:26 -08:00
epriestley
f1a035d5c2 In Differential, give the "moved/copied from" gutter a more clear visual look
Summary:
Depends on D20196. See PHI985. When empty, the "moved/copied" gutter currently renders with the same background color as the rest of the line. This can be misleading because it makes code look more indented than it is, especially if you're unfamiliar with the tool:

{F6225179}

If we remove this misleading coloration, we get a white gap. This is more clear, but looks a little odd:

{F6225181}

Instead, give this gutter a subtle background fill in all casses, to make it more clear that it's a separate gutter region, not a part of the text diff:

{F6225183}

Test Plan: See screenshots. Copied text from a diff, added/removed inlines, etc.

Reviewers: amckinley

Reviewed By: amckinley

Differential Revision: https://secure.phabricator.com/D20197
2019-02-20 10:12:16 -08:00
epriestley
a33409991c Remove an old Differential selection behavior
Summary:
Ref T12822. Ref PHI878. This is some leftover code from the old selection behavior that prevented visual selection of the left side of a diff if the user clicked on the right -- basically, a much simpler attack on what ultimately landed in D20191.

I think the change from `th` to `td` "broke" it so it didn't interfere with the other behavior, which is why I didn't have to remove it earlier. It's no longer necessary, in any case.

Test Plan: Grepped for behavior name, selected stuff on both sides of a diff.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T12822

Differential Revision: https://secure.phabricator.com/D20196
2019-02-20 10:09:34 -08:00
epriestley
cf048f4402 Tweak some display behaviors for indent indicators
Summary:
Ref T13161.

  - Don't show ">>" when the line indentation changed but the text also changed, this is just "the line changed".
  - The indicator seems a little cleaner if we just reuse the existing "bright" colors, which already have colorblind colors anyway.

Test Plan: Got slightly better rendering for some diffs locally.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13161

Differential Revision: https://secure.phabricator.com/D20195
2019-02-19 15:34:30 -08:00
epriestley
efccd75ae3 Correct various minor diff copy behaviors
Summary:
Ref T12822. Fixes a few things:

  - Firefox selection of weird ranges with an inline between the start and end of the range now works correctly.
  - "Show More Context" rows now render, highlight, and select properly.
  - Prepares for nodes to have copy-text which is different from display-text.
  - Don't do anything too fancy in 1-up/unified mode. We don't copy line numbers after the `content: attr(data-n)` change, but that's as far as we go, because trying to do more than that is kind of weird and not terribly intuitive.

Test Plan:
  - Selected and copied weird ranges in Firefox.
  - Kept an eye on "Show More Context" rows across select and copy operations.
  - Generally poked around in Safari/Firefox/Chrome.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T12822

Differential Revision: https://secure.phabricator.com/D20192
2019-02-19 15:18:45 -08:00
epriestley
37f12a05ea Behold! Copy text from either side of a diff!
Summary:
Ref T12822. Ref T13161. By default, when users select text from a diff and copy it to the clipboard, they get both sides of the diff and all the line numbers. This is usually not what they intended to copy.

As of D20188, we use `content: attr(...)` to render line numbers. No browser copies this text, so that fixes line numbers.

We can use "user-select" CSS to visually prevent selection of line numbers and other stuff we don't want to copy. In Firefox and Chrome, "user-select" also applies to copied text, so getting "user-select" on the right nodes is largely good enough to do what we want.

In Safari, "user-select" is only visual, so we always need to crawl the DOM to figure out what text to pull out of it anyway.

In all browsers, we likely want to crawl the DOM anyway because this will let us show one piece of text and copy a different piece of text. We probably want to do this in the future to preserve "\t" tabs, and possibly to let us render certain character codes in one way but copy their original values. For example, we could render "\x07" as "␇".

Finally, we have to figure out which side of the diff we're copying from. The rule here is:

  - If you start the selection by clicking somewhere on the left or right side of the diff, that's what you're copying.
  - Otherwise, use normal document copy rules.

So the overall flow here is:

  - Listen for clicks.
  - When the user clicks the left or right side of the diff, store what they clicked.
  - When a selection starts, and something is actually selected, check if it was initiated by clicking a diff. If it was, apply a visual effect to get "user-select" where it needs to go and show the user what we think they're doing and what we're going to copy.
  - (Then, try to handle a bunch of degenerate cases where you start a selection and then click inside that selection.)
  - When a user clicks elsewhere or ends the selection with nothing selected, clear the selection mode.
  - When a user copies text, if we have an active selection mode, pull all the selected nodes out of the DOM and filter out the ones we don't want to copy, then stitch the text back together. Although I believe this didn't work well in ~2010, it appears to work well today.

Test Plan: This mostly seems to work in Safari, Chrome, and Firefox. T12822 has some errata. I haven't tested touch events but am satisfied if the touch event story is anything better than "permanently destroys data".

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13161, T12822

Differential Revision: https://secure.phabricator.com/D20191
2019-02-19 15:17:07 -08:00
epriestley
d4b96bcf6b Remove hidden zero-width spaces affecting copy behavior
Summary:
Ref T13161. Ref T12822. Today, we use invisible Zero-Width Spaces to try to improve copy/paste behavior from Differential.

After D20188, we no longer need ZWS characters to avoid copying line numbers. Get rid of these secret invisible semantic ZWS characters completely.

This means that both the left-hand and right-hand side of diffs become copyable, which isn't desired. I'll fix that with a hundred thousand lines of Javascript in the next change: this is a step toward everything working better, but doesn't fix everything yet.

Test Plan:
  - Grepped for `zws`, `grep -i zero | grep -i width`.
  - Copied text out of Differential: got both sides of the diff (not ideal).

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13161, T12822

Differential Revision: https://secure.phabricator.com/D20189
2019-02-19 15:10:04 -08:00
epriestley
98fe8fae4a Use <td class="n" data-n="3"> instead of <th>3</th> for line numbers
Summary:
Ref T13161. Ref T12822. See PHI870. Long ago, the web was simple. You could leave your doors unlocked, you knew all your neighbors, crime hadn't been invented yet, and `<th>3</th>` was a perfectly fine way to render a line number cell containing the number "3".

But times have changed!

  - In PHI870, this isn't good for screenreaders. We can't do much about this, so switch to `<td>`.
  - In D19349 / T13105 and elsewhere, this `::after { content: attr(data-n); }` approach seems like the least bad general-purpose approach for preventing line numbers from being copied. Although Differential needs even more magic beyond this in the two-up view, this is likely good enough for the one-up view, and is consistent with other views (paste, harbormaster logs, general source display) where this technique is sufficient on its own.

The chance this breaks //something// is pretty much 100%, but we've got a week to figure out what it breaks. I couldn't find any issues immediately.

Test Plan:
  - Created, edited, deleted inlines in 1-up and 2-up views.
  - Replied, keyboard-navigated, keyboard-replied, drag-selected, poked and prodded everything.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13161, T12822

Differential Revision: https://secure.phabricator.com/D20188
2019-02-19 15:07:02 -08:00
epriestley
661c758ff9 Render indent depth changes more clearly
Summary:
Ref T13161. See PHI723. Our whitespace handling is based on whitespace flags like `diff -bw`, mostly just for historical reasons: long ago, the easiest way to minimize the visual impact of indentation changes was to literally use `diff -bw`.

However, this approach is very coarse and has a lot of problems, like detecting `"ab" -> "a b"` as "only a whitespace change" even though this is always semantic. It also causes problems in YAML, Python, etc. Over time, we've added a lot of stuff to mitigate the downsides to this approach.

We also no longer get any benefits from this approach being simple: we need faithful diffs as the authoritative source, and have to completely rebuild the diff to `diff -bw` it. In the UI, we have a "whitespace mode" flag. We have the "whitespace matters" configuration.

I think ReviewBoard generally has a better approach to indent depth changes than we do (see T13161) where it detects them and renders them in a minimal way with low visual impact. This is ultimately what we want: reduce visual clutter for depth-only changes, but preserve whitespace changes in strings, etc.

Move toward detecting and rendering indent depth changes. Followup work:

  - These should get colorblind colors and the design can probably use a little more tweaking.
  - The OneUp mode is okay, but could be improved.
  - Whitespace mode can now be removed completely.
  - I'm trying to handle tabs correctly, but since we currently mangle them into spaces today, it's hard to be sure I actually got it right.

Test Plan: {F6214084}

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13161

Differential Revision: https://secure.phabricator.com/D20181
2019-02-19 12:40:05 -08:00
epriestley
09c6d42b95 Mostly make blame work with DocumentEngine
Summary: Ref T13105. This needs refinement but blame sort of works again, now.

Test Plan: Viewed files in Diffusion and Files; saw blame in Diffusion when viewing in source mode.

Reviewers: mydeveloperday

Reviewed By: mydeveloperday

Maniphest Tasks: T13105

Differential Revision: https://secure.phabricator.com/D19309
2018-04-09 04:48:21 -07:00
Chad Little
2f26dd76de Lots of little fixes for Dark Mode (Experimental)
Summary: Cleans up a bunch of Differential odd/special colors. Adds some basic "highlight" colors instead of pure yellow.

Test Plan: Test each color change in normal and dark modes.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D18239
2017-07-19 14:41:23 -07:00
Chad Little
7aeefc0cca Add an Experimental Dark Mode to Phabricator
Summary: Mostly this is an exercise to clean up our CSS and Celerity processor by making sure all important color decisions are generatable. It's somewhat resonable to use if you don't review code. Posting it up here mostly so I don't lose the work.

Test Plan: Visit lots and lots of pages with dark mode on and off.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D18227
2017-07-18 06:44:32 -07:00
Chad Little
565c49ad0e Minor touchup on diff banner
Summary: Remove extra icon spacing, swap icons.

Test Plan: Review a diff with comments in sandbox. Try dropdown. Follow links

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D18216
2017-07-12 15:23:14 -07:00
epriestley
3be36783b3 Consider inline comments with draft checkmarks as "unsubmitted"
Summary:
Ref T12733. When a revision has unsubmitted checkmarks:

  - Color the banner yellow.
  - Show them in the "X unsubmitted" count.
  - Make the "X unsubmitted" button cycle between all drafts (written but unpublished comments) and "draft done" (checked but unsubmitted "Done" checkbox comments).

Test Plan:
  - Checked a "Done" box, saw "1 unsubmitted" and yellow banner.
  - Clicked "5 unsubmitted" repeatedly, saw it cycle through all unsubmitted comments and checkboxes.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12733

Differential Revision: https://secure.phabricator.com/D18127
2017-06-15 05:22:58 -07:00
Chad Little
0d1654446b Clean up spacing on diff-banner
Summary: Adds spacing to the buttons, line-height for aligning text vertically.

Test Plan: Leave comments on a diff.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D18110
2017-06-09 22:17:36 +00:00
epriestley
48c6ca40c4 Add an "Unsaved" button to the Differential persistent header
Summary: Ref T12733.

Test Plan: {F4987956}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12733

Differential Revision: https://secure.phabricator.com/D18070
2017-06-05 09:48:08 -07:00
epriestley
83e99fb691 Add a class to the Differential banner when unsubmitted/unsaved changes are present
Summary: Ref T12733. This adds classes for unsubmitted/unsaved changes, and lays some groundwork for additional buttons.

Test Plan:
  - Added, edited, deleted comments.
  - Saw bar background color update appropriately.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12733

Differential Revision: https://secure.phabricator.com/D18045
2017-05-30 17:59:23 -07:00
epriestley
d20221dc7d Remove Differential "objectives" UI
Summary: Ref T12733. Completely removes the objectives UI.

Test Plan:
  - Grepped for `objective`, etc.
  - Browsed revisions, no JS errors / broken stuff.
  - (If I missed anything, it's likely to turn up in followup changes.)

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12733

Differential Revision: https://secure.phabricator.com/D18043
2017-05-30 17:58:49 -07:00
epriestley
af07600aaa Make Differential objective markers show a brighter "editing" state
Summary:
Ref T12733.

  - While editing a comment, show a pink star ({icon star, color=pink}) with a tooltip.
  - Slight UI tweaks, including draft comments getting an indigo pencil ({icon pencil, color=indigo}).

Test Plan: {F4968470}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12733

Differential Revision: https://secure.phabricator.com/D17977
2017-05-20 07:57:38 -07:00
epriestley
aba209e999 Hide the Differential scroll objective list on trackpad systems
Summary:
Ref T12733. In the longer run I'd like to just push this out from the edge, but that currently gets us into trouble since we start bumping into content. On my system, the trackpad scrollbar also expands in size when moused over, so the minimum number of pixels we need to push it out is approximatley 15px. This hits body content and the persistent chat.

For now, just disable this element on trackpad systems.

Test Plan:
Disconnected all USB peripherals, quit and relaunched Safari, saw no objective list.

Reconnected mouse, relaunched Safari, saw objective list.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12733

Differential Revision: https://secure.phabricator.com/D17974
2017-05-20 07:56:21 -07:00
epriestley
fdf00f6df4 Clean up some minor UI behaviors in Differential
Summary:
Minor UI tweaks:

  - Use the dynamic icon for each file (e.g., image, text), not a hard-coded icon.
  - Render the path (less important) in grey and the filename (more important) in black.

Test Plan: {F4966176}

Reviewers: chad

Reviewed By: chad

Differential Revision: https://secure.phabricator.com/D17957
2017-05-19 12:01:58 -07:00
epriestley
6c46f27d98 Add quest objectives to the minimap
Summary:
Add important objectives (like waygates and quest markers) to the minimap.

This also probably fixes @cspeckmim's bug with the {key @} keyboard shortcut.

Test Plan:
(This is probably easier to undestand if you `arc patch` + click around.)

{F4966037}

Reviewers: chad, amckinley

Reviewed By: chad

Subscribers: cspeckmim

Differential Revision: https://secure.phabricator.com/D17955
2017-05-19 12:01:01 -07:00
epriestley
fb9f3cc0b4 Restore the "buoyant" header in Differential
Summary:
Fixes T1591. This was removed long ago because it was a mess to implement and caused a bunch of weird issues, and also my tolerance for dealing with weird JS issues was much, much lower.

I have now survived the fires of JX.Scrollbar and would love to address 200 small nitpicks about obscure browser behaviors on Linux, so open the floodgates again.

A secondary goal here is to create room to add a global view state menu on the right, with 300 options like "hide all inlines", "hide done inlines", "hide collapsed inlines", "hide ghosts", "show ghosts", "enable filetree", "disable filetree", etc, etc. Not sure how much of this I'll actually do. I have one more experiment I want to try first.

Test Plan: {F4963294}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T1591

Differential Revision: https://secure.phabricator.com/D17945
2017-05-18 10:24:26 -07:00
epriestley
f78ce156f1 Restore "h" to hide or show files, and modernize file visibility toggling
Summary:
Ref T12616. This puts "h" back to collapse or expand the current file.

This removes some very complicated/messy code around following links in the table of contents and getting files auto-expanded. I suspect no one will miss this, but we can restore it if ayone notices.

Test Plan: Pressed "h" to collapse/expand a file. Also used the menu items.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12616

Differential Revision: https://secure.phabricator.com/D17940
2017-05-18 10:21:37 -07:00
epriestley
0ca49fbeb9 Move "hover over an inline to see the affected lines" code to the new class tree
Summary:
Fixes T8047. Ref T12616. Fixes T9270. This moves the "hover" part of the hover/drag behavior to the new code, leaving the "drag" part for a followup change.

The new hover UI behaves properly with Quicksand (T8047) and the filetree (T9270).

Test Plan: Hovered over inlines, saw lines select properly.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12616, T9270, T8047

Differential Revision: https://secure.phabricator.com/D17919
2017-05-16 17:37:38 -07:00
epriestley
772afc5ed8 Allow cancelled inlines, edits, and replies to be undone to get the text back again
Summary: Ref T12616. The ability to do {nav Edit > Cancel > Undo} to get your text back on inlines got dropped during the conversion. Restore it.

Test Plan:
Created, replied, and edited inlines, typed text, then cancelled. Was able to undo.

Also undid normal deletion.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12616

Differential Revision: https://secure.phabricator.com/D17916
2017-05-16 13:12:14 -07:00
epriestley
6a9dd61c42 Make collapsed inlines more useful and anchor target highlights more accurate
Summary:
Ref T12616. Fixes T11648. Currently, we snug up replies with a negative margin (from T10563) but this throws off the anchor highlighting.

Instead:

  - Remove padding from these dolumns.
  - Use margins on the stuff inside them instead.
  - Less margins for replies.
  - Less margins for collapsed comments.
  - Show some text for collapsed comments.

Test Plan:
{F4960890}

{F4960891}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12616, T11648

Differential Revision: https://secure.phabricator.com/D17913
2017-05-16 11:09:53 -07:00
epriestley
4fd4ec3d27 Hide inlines one-by-one, instead of in a big group
Summary:
Ref T12616. Fixes T12153. Currently, when you hide inlines, they hide completely and turn into a little bubble on the previous line.

Instead, collapse them to a single line one-by-one. Narrowly, this fixes T12153.

In the future, I plan to make these changes so this feature makes more sense:

  - Introduce global "hide everything" states (T8909) so you can completely hide stuff if you want, and this represents more of a halfway state between "nuke it" and "view it".
  - Make the actual rendering better, so it says "epriestley: blah blah..." instead of just "..." -- and looks less dumb.

The real goal here is to introduce `DiffInline` and continue moving stuff from the tangled jungle of a million top-level behaviors to sensible smooth statefulness.

Test Plan:
  - Hid and revealed inlines in unified and two-up modes.
  - These look pretty junk for now:

{F4948659}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T12616, T12153

Differential Revision: https://secure.phabricator.com/D17861
2017-05-16 06:19:56 -07:00
Chad Little
1e1a0182ca Add basic diff coloring to CelerityDefaultProcessor
Summary: Moves basic colors into the processor.

Test Plan: Review a diff in sandbox with and without change.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D17173
2017-01-10 17:54:24 -08:00
Chad Little
63af2275ca Re-darken full width diff colors
Summary: Ref T12089. This reverts back to using "bright" colors for full width changes in green and red.

Test Plan:
Review diffs, see full width colors.

{F2333933}

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T12089

Differential Revision: https://secure.phabricator.com/D17171
2017-01-10 13:25:23 -08:00
Chad Little
96fbf37dcc Bring up contrast on light green / red diffs
Summary: Minor color saturation here, ideal for low quality monitors.

Test Plan:
Review new colors in various scenarios.

{F2305178}

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D17141
2017-01-04 15:18:24 -08:00
Chad Little
aa9708c5d3 Update diff highlight colors for better color blindess distinction
Summary: Tweaks the diff colors here a bit, as well as making full diffs slightly easier to read in full. Ref T12060

Test Plan:
Tested prose diffs, email prose diffs, and a regular Differential revision.

{F2304056}

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T12060

Differential Revision: https://secure.phabricator.com/D17138
2017-01-04 11:35:19 -08:00
epriestley
3f5109b668 In prose diff dialogs (like "Show Details" in transactions), show "old", "new" and "diff" tabs
Summary:
Ref T7643. When you do something like this:

  - Edit a task description.
  - Click "Show Details" on the resulting transaction.
  - Get a prose diff dialog showing the change.

...now add some "Old" and "New" tabs. These are useful for:

  - reverting to the old text by copy/pasting;
  - reading just the new/old text if the diff is noisy;
  - sometimes just nice to have?

(This looks a little rough but I didn't want to put a negative margin on tab groups inside dialogs? Not sure what the best fix here is.)

Test Plan: {F1909390}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T7643

Differential Revision: https://secure.phabricator.com/D16817
2016-11-07 15:18:19 -08:00
Chad Little
e2d195e03a Fix mobile menu for files in Differential Changesets
Summary: Fixes T11305, Ref T7754. Makes this menu dropdown act like actions and collapse to a fa-bars menu.

Test Plan:
View on mobile, desktop, browser. Click an action, spawn new page.

{F1717953}

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T7754, T11305

Differential Revision: https://secure.phabricator.com/D16265
2016-07-09 14:25:48 -07:00
epriestley
84ce863116 Select <td> padding for inlines more surgically
Summary:
Fixes T11225. The primary issue here is that this rule is bleeding down too far. It appears that it's only intended to put space around the inline as a whole.

Spacing still isn't //perfect// since a few other rules are bleeding, but it feels reasonable now instead of being clearly broken.

Test Plan:
  - Added "background: red;" to figure out what was being affected.
  - Before:

{F1704081}

  - After:

{F1704084}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T11225

Differential Revision: https://secure.phabricator.com/D16184
2016-06-28 14:50:51 -07:00
epriestley
fb156af480 Render prose diffs in email as summaries
Summary:
Ref T7643. When a large block of prose text is edited (like a wiki page), summarize the diff when sending mail.

For now, I'm still showing the whole thing in the web UI, since it's a bit more manageable there.

Also try to fix newlines in Airmail.

Test Plan:
This web diff:

{F1682591}

..became this mail diff:

{F1682592}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T7643

Differential Revision: https://secure.phabricator.com/D16098
2016-06-10 09:39:58 -07:00
epriestley
02877c600a Use prose diffs in Phriction
Summary: Ref T3353. This improves whitespace display of prose diffs and uses them in Phriction.

Test Plan: {F1677957}

Reviewers: chad, avivey

Reviewed By: avivey

Subscribers: avivey

Maniphest Tasks: T3353

Differential Revision: https://secure.phabricator.com/D16062
2016-06-06 14:33:23 -07:00
epriestley
b3477bfc56 Render sometimes-legible prose diffs in the Phabricator UI
Summary:
Ref T3353. This hooks the prose engine up to the UI and throws away the hard-wrapping hacks.

These are likely still very rough in many cases, but are hopefully a big step forward from the old version in the vast majority of cases.

Test Plan: {F1677809}

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T3353

Differential Revision: https://secure.phabricator.com/D16056
2016-06-06 12:33:42 -07:00
Chad Little
2dc4096ea1 Lighten up red and green in diffs
Summary: Because new+old stack, these colors were darker than intended. Lightening them up a little bit.

Test Plan: Review a diff with new + old code and addtion subtractions.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D15994
2016-05-31 20:25:28 +00:00
Chad Little
03a1deba23 Remove hard-coding of diff line height
Summary: Fixes T10959. This is the smallest/simplest fix that I could come up with, and I wasn't able to break it. Basically, I removed "line-height" and then adjusted other rules until the defaults looked reasonable again.

Test Plan:
Here's `24px / 48px impact` or something like it:

{F1310445}

{F1310446}

Here's normal stuff working properly without weird artifacts on the highlighting:

{F1310447}

Also tested Firefox and Chrome and got similar results.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: wxm20073527, Korvin

Maniphest Tasks: T10959

Differential Revision: https://secure.phabricator.com/D15905
2016-05-16 10:21:37 -07:00
Chad Little
148a50e48b Convert Differential to new layout
Summary:
First pass at converting Differential, I likely have some buggy-poos but thought I'd toss this up now in case very bad bugs present.

To do:
- Need to put status back on Hovercards
- "Diff Detail" probably needs a better design

Test Plan: Looking at lots of diffs, admittedly I dont have harbormaster, etc, running locally. Checked Diffusion for Table of Content changes on small and large commits.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D15463
2016-03-12 13:04:21 -08:00
epriestley
68b468a846 Partially improve threading UI for adjacent inline comments
Summary:
Ref T10563. This isn't a complete fix, but should make viewing complex inline threads a little more manageable.

This just tries to put stuff in thread order instead of in pure chronological order. We can likely improve the display treatment -- this is a pretty minimal approach, but should improve clarity.

Test Plan:
T10563 has a "before" shot. Here's the "after":

{F1169018}

This makes it a bit easier to follow the conversations.

Reviewers: chad

Reviewed By: chad

Maniphest Tasks: T10563

Differential Revision: https://secure.phabricator.com/D15459
2016-03-10 17:40:13 -08:00
Chad Little
0e5cd478c4 Move rgba rules into CelerityDefaultPostprocessor
Summary: Should make it simpler here to have more `rgba` rules in CSS for things like hovers, selected states. Maybe only use `rgb` colors? Color pallette probably needs an overhaul.

Test Plan: Bounce around random pages, buttons, menus. Everything appears normal.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D15273
2016-02-16 09:54:43 -08:00
Chad Little
c9de213750 Fix mobile previous revision inline comments
Summary: Fixes T9016, adds additonal space around icons.

Test Plan: Visit an old inline comment, no overlapping icon.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: Korvin

Maniphest Tasks: T9016

Differential Revision: https://secure.phabricator.com/D13914
2015-08-16 17:53:02 -07:00
Chad Little
3b755115c2 [Redesgin] Misc Inline Comment tweaks
Summary: Ref T8099. Better alignment with TODO and Ghost Icons.

Test Plan: Test more states of inline comments, ghosts, todos, unsubmitted, etc.

Reviewers: btrahan, epriestley

Reviewed By: epriestley

Subscribers: epriestley, Korvin

Maniphest Tasks: T8099

Differential Revision: https://secure.phabricator.com/D13515
2015-07-01 11:32:32 -07:00
Chad Little
6fef37ddb5 [Redesign] Update Inline Comment UI
Summary: Ref T8099, Simplifies the button bar with a `borderless` option and implements in Differential Inline Commenting.

Test Plan:
Review new and old comments, submitted, unsubmitted, ghosts, done.

{F562765}

Reviewers: btrahan, epriestley

Reviewed By: epriestley

Subscribers: epriestley, Korvin

Maniphest Tasks: T8099

Differential Revision: https://secure.phabricator.com/D13475
2015-06-29 14:42:29 -07:00
Chad Little
1c84d2719c [Redesign] Larger Font PostProcessor
Summary: Ref T8099, Ref T8614. Reasonable attempt at a larger font size PostProcessor, will need additional UI cleanup (places where we fix height) but overall very decent.

Test Plan: Use lots of the UI elements with new font sizes.

Reviewers: btrahan, epriestley

Reviewed By: epriestley

Subscribers: epriestley, Korvin

Maniphest Tasks: T8099, T8614

Differential Revision: https://secure.phabricator.com/D13457
2015-06-26 09:33:03 -07:00
Chad Little
1508b538a4 [Redesign] Clean up differential file headers
Summary: Ref T8099, normalizes font-weight on the Differential file headers.

Test Plan: Review a diff in sandbox for correct font weight.

Reviewers: btrahan, epriestley

Reviewed By: epriestley

Subscribers: epriestley, Korvin

Maniphest Tasks: T8099

Differential Revision: https://secure.phabricator.com/D13438
2015-06-25 07:53:55 -07:00
epriestley
075f041bec Merge branch 'master' into redesign-2015 2015-06-23 13:36:47 -07:00