Summary: See some discussion in D5622. Javelin explicitly prevents you from putting `<script>` tags into `JX.$H()`, which is probably a good idea, so just let this behavior fail less abruptly instead. We currently have no cases where we load something into a cache and then make a decision about whether to put it into the document or not later on; this should hold us over until we do. If and when we do, we can let those endpoints capture behaviors and replay them later or something.
Test Plan: Verified tokenizers still work correctly.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D5623
Summary:
the JS is fragile with respect to the tokenizer coming in from the people widget. make sure to always try to load this up. Note this generally needs to get cleaned up where the server should only send down the *exact* bits the client needs. This is all TODO as part of getting this on mobile perfectly. Also note this fragility exists still in that you can break conpherence by clicking quickly before the initial tokenizer load loads.
For old bad data, at some point we weren't updating participation as well as we do today in the editor class. the result is with the migration and code change some conversation participants have bad "last seen message" counts. the simplest case is the test user talking to themselves -- threads before the editor code fixes / changes will have the entire thread as unread for these folks. The other buggy case I saw was where the "last reply" to a thread wasn't being count. These issues showed up for threads February and older which is old.
Test Plan: edited conpherence meta data and no JS bugs. pontificated and no JS bugs. added a person and no JS bugs.
Reviewers: chad, epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5622
Summary: also re-enables the updating of the widgets and "cleans up" the javascript a tad. Ref T2867
Test Plan: all sorts of conpherence fun like adding people to threads, adding files, pontificating, etc
Reviewers: epriestley, chad
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2867
Differential Revision: https://secure.phabricator.com/D5595
Summary:
Refs T1048; Fixes T2902 - (Probably) fixes @vrana's issue. I managed
to repro it on Ubuntu FF (though on Windows with 1.0GHz/512MB it's
really worse...).
This revises the approach to the graceful degradation for
`too-far-to-the-screen-edge-edge-case`.
I noticed that `x` could become very negative, up to about ~`-170px`.
This is due to the //"already-on-the-left-side"// nature of object tags.
`50 - 200 - 20` = `negative`. Adding `100px` (node.dimension.x / 4) to
that won't really help, as the hovercard would still be offscreen.
Instead, display them left-aligned with the object tags on the left edge
per default, and offer centerization in center cases. This is also better
for Pholio, Phriction, which have a way lower min-x than Maniphest,
Differential.
I also disabled placing the hovercard below the tag in case there's not
enough space on the north side. The hovercard would not display 99.99% of
the times after being hidden (and it retains the flickering behaviour).
Another reason is also our current hide-behaviour, which only assumes
north-side alignment. Adding south-side didn't really work (I'm bad with
JS), so I didn't bother with it. Disabling this is //acceptable//, since
it only really affects Pholio, Phriction. And nobody places object tags
in the first line, anyway. Except for my test pages, of course :/
Btw, this also removes the weird jaggy horizontal shifts for object of
various lengths (e.g. `{D4356}`, `{rP32ofhw0842obw}` etc.). I think
that's only good.
Test Plan:
Hovered in Pholio, Phriction in Chrome, FF. Did not touch
left screen border.
Hovered in Maniphest, Differential. Tags farther to the left were
aligned left, tags more in the center were pretty centered.
Reviewers: epriestley, vrana
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T1048, T2902
Differential Revision: https://secure.phabricator.com/D5612
Summary: Simple alternative to D5448. Adds a "header" type which renders a visual separator.
Test Plan:
{F39507}
{F39508}
Reviewers: jamesr, btrahan, chad
Reviewed By: chad
CC: aran
Differential Revision: https://secure.phabricator.com/D5616
Summary: Tightens up spacing, remove some of the borders, add alpha channel, make them all blue (sorry, red green and yellow are for 'status'). If we want to do more colors just for hovercards, I have a brown and a black in the mock, but would like to try just blue for now.
Test Plan: UIExamples, Tasks, People, Diffs, and Pastes.
Reviewers: epriestley, AnhNhan, btrahan
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5609
Summary:
Refs T1048, T2902 - This //should// fix flickering hovercards. Probably does not fix displaced hovercards, though could (because of the flickering doing something bad to rendering).
I'm bad with JS D:
Test Plan: Tried out plenty of reload & hover combos. No flickering anymore. Never again.
Reviewers: epriestley, vrana
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T1048
Differential Revision: https://secure.phabricator.com/D5607
Summary: At least for non-workboard views, try plain text for author information instead of profile images. Some discussion in D5451.
Test Plan:
{F39411}
{F39412}
{F39413}
{F39414}
{F39415}
{F39416}
Reviewers: chad
Reviewed By: chad
CC: AnhNhan, aran
Differential Revision: https://secure.phabricator.com/D5605
Summary: Refs T1048 - disables bg repeat, centers profile image
Test Plan: looked at my own hovercard in my local install, which has a non-square prof pic
Reviewers: epriestley, chad
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T1048
Differential Revision: https://secure.phabricator.com/D5604
Summary:
Refs T1048 - Take this setup:
D123D321
Hover with the mouse over one object, and move to another one before it got loaded. You'll have two hovercards. Of these you can't get rid of one anymore. Not through resizing, not through showing another hovercard. (You may have to try a few times). I think it only happens when #2 loads faster than #1. #2 is displayed, then #1 gets loaded and draws.
Or something like that. You could have a race condition where one draws after another and overwrites the current node/anchor, without hiding it. This renders the previous card unhideable, even on resize / mouse far away.
Test Plan:
Did a repro by hovering a lot.
Applied this change. Could not repro anymore.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T1048
Differential Revision: https://secure.phabricator.com/D5594
Summary: See discussion in D5563. This loads hovercards on-demand, and shows them once they load. Ref T1048.
Test Plan: Made a preview comment like `T1`, waved my mouse over it, got a hovercard shortly thereafter.
Reviewers: AnhNhan
Reviewed By: AnhNhan
CC: aran
Maniphest Tasks: T1048
Differential Revision: https://secure.phabricator.com/D5588
Summary:
Refs T1048; Depends on D5557, D5558
They hover right above your eyes. Try it out at home :D (in that case, don't forget to checkout D5557 and D5558)
Worth a lot of improvement. But it's great for now after a little bit of styling.
Scrape load (search current document for all hovercards and pre-load them) and lazy load (e.g. previewing comments which is not covered by scrape load) implemented.
Added some seemingly useful graceful situations. Too much to the left, too much to the top.
Test Plan:
Tested on Ubuntu, Chrome and FF. No Windows yet, since I have it live at no place. Works pretty fine, at least it will appear.
Could test left graceful fallback. Don't know what happens if you place it too far to the top. I expect either good results or placement about the center of the screen in case of glitches.
For now, they'll disappear right away once the mouse leaves the object tag. Intended is when leaving both tag and hovercard.
Reviewers: epriestley, chad, btrahan
CC: aran, Korvin
Maniphest Tasks: T1048
Differential Revision: https://secure.phabricator.com/D5563
Conflicts:
src/__celerity_resource_map__.php
Summary: Initial pass at an action header. The idea is to support current and future planned needs in 'headers' with various colors and icons. The overall goal here is to keep markup light and allow other classes to wrap and extend with more specific features.
Test Plan: Tested UIExamples and Workboards.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin, AnhNhan
Differential Revision: https://secure.phabricator.com/D5585
Test Plan: go to the new task page, go to the 'Assign To' field, assign the task to someone and try to do a Shift-Tab to return to the title field
Reviewers: igoleo, btrahan
Reviewed By: btrahan
CC: aran, Korvin
Maniphest Tasks: T2760
Differential Revision: https://secure.phabricator.com/D5547
Summary: First pass at 'action icons' for headers and other items. Ties into future diff.
Test Plan: photoshop, read the css.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin, AnhNhan
Differential Revision: https://secure.phabricator.com/D5565
Summary: Ref T2400 and M14.
Test Plan: had a few conpherences with various status set amongst participants; views were sensical.
Reviewers: epriestley, chad
Reviewed By: chad
CC: aran, Korvin
Maniphest Tasks: T2400
Differential Revision: https://secure.phabricator.com/D5541
Summary: Makes the width fluid under mobile devices.
Test Plan: Test in small Chrome and iOS.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5556
Summary: Pulls in the top 2px and left 2px. Also reduces to 13px as standard. The 14px never looked decent to me. Main objective here is to have it feel more table-like instead of a sea of white space.
Test Plan: Tested layouts in Maniphest, Config, and UIExamples.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5551
Summary: Removes the glow icons and uses a hover change. Fixes phantom anchors.
Test Plan: Review in Chrome at various sizes (phone tablet). Check that icons still work. Check that mobile menus render when clicked.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2876
Differential Revision: https://secure.phabricator.com/D5549
Summary:
Apparently I am crazy and didn't test D5537 propertly at all. In particular:
- Currently, the update sends back new "people" and "files" widgets. The "people" widget has a tokenizer, which fatals when the behavior initializes without the widget in the DOM. For now, disable widget updates on replies. I'll fix this in a future diff.
- Currently, we don't update the "last_transaction_id" in the form itself, so the first reply sends back 1 message, the next 2 messages, etc. Update the input.
- The transaction paging doesn't and has never worked, I am crazy. Make it actually work.
Test Plan:
computers are too hard
(also, this is why I hate Javascript)
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D5538
Summary: Abstract out the multi-column code from workboards and have it be available separately. I feel like there will be some benefit here especially for custom developers in how they present infromation (like releeph). It also scales back to tablet and mobile fairly well, so they get those things for free.
Test Plan: Tested mobile, tablet and chrome layouts.
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5529
Summary: Fixes T2861. This used to work but I think I broke it when I made the notification popup thing work. Merge it into the drag-and-drop since 90% of the code is the same anyway.
Test Plan: Pasted files into Chrome and got uploads. This doesn't work in Safari and Firefox; as far as I know it isn't supported in those browers.
Reviewers: blc, btrahan, chad
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2861
Differential Revision: https://secure.phabricator.com/D5530
Summary:
adds ye olde people widget. Features include
- handle-based display, so we get status for free. (Note less pretty than M14 would have it!)
- can add a person
- can remove a person
- can see the people already in the conpherence
Test Plan: added and removed people and noted they joined / re-added as appropriate. Tried to add someone already in the conpherence and got a "transaction has no effect" message
Reviewers: epriestley, chad
Reviewed By: epriestley
CC: aran, Korvin
Maniphest Tasks: T2399
Differential Revision: https://secure.phabricator.com/D5466
Summary:
Refs T1048 - I'm pretty happy and happy to tell you
Savepoint CR
Everything's dummy right now. If you are testing locally, don't forget to edit the PHIDs. Or populate your own handles. Doesn't really matter.
I'm mainly sending it in for the CSS, not the messy PHP code. Ignore the `margin: auto`, that's just for looking nice.
Test Plan: UI Example » Hovercard
Reviewers: epriestley, chad
CC: aran, Korvin
Maniphest Tasks: T1048
Differential Revision: https://secure.phabricator.com/D5519
Summary: Good that this isn't a water leak. Else I'd be screwed.
Test Plan:
Visited Main Dir. Looks normal.
Visited Maniphest. Saw ominous shadow on top for saved query. Saw normal paddings for populated list filters.
Reviewers: epriestley, chad
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5536
Summary: Fixes T2811. Chrome requires this, but only for files dragged from the download shelf, I guess? Not entirely sure what's going on here. I couldn't find much documentation on this and figured it out mostly by process of elimination.
Test Plan: Drag and drop still works everywhere, plus from the Chrome download shelf.
Reviewers: blc, btrahan, skrul
Reviewed By: skrul
CC: aran
Maniphest Tasks: T2811
Differential Revision: https://secure.phabricator.com/D5528
Summary: We may execute the Conpherence behavior before the initial device change, in which case we'll get a desktop -> desktop device event. This currently causes us to double-load the thread list. Instead, don't do anything if the device is the same as our current understanding of device state.
Test Plan: No double thread list in profiler.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D5525
Summary:
- Move `/N/` to `/thread/N/`.
- Move `/view/N/` to `/N`/.
- This makes the mobile "Conpherence -> click thread -> see thread" workflow work correctly. This also makes permalinks work correctly on mobile.
Ref T2421.
Test Plan: Clicked flows on desktop, mobile.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2421
Differential Revision: https://secure.phabricator.com/D5508
Summary:
When we access `/conpherence/view/2/` on the desktop, load the thread on the initial response and ajax in the thread list. Basically, the idea is:
- When we load the thread list, an individual thread, or the widget panel, we send back the piece the user asked for in the response.
- On mobile, we stop there: we don't ajax in anything else, and just hide the other parts of the layout.
- On Desktop, we fill out the other layout components via Ajax.
Ref T2421.
Test Plan: Hit `/conpherence/view/2/`, got a full page.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2421
Differential Revision: https://secure.phabricator.com/D5507
Summary: The actual layout on mobile is a bit silly since the thread ends up being like 5px tall for now, but it technically works. Ref T2644.
Test Plan: {F38106}
Reviewers: btrahan
Reviewed By: btrahan
CC: aran, chad
Maniphest Tasks: T2421
Differential Revision: https://secure.phabricator.com/D5506
Summary: Currently, `/conpherence/` shows the widget panel on devices. Make it show the thread list instead.
Test Plan: {F38099}
Reviewers: btrahan
Reviewed By: btrahan
CC: aran, chad
Maniphest Tasks: T2421
Differential Revision: https://secure.phabricator.com/D5505
Summary: Currently, the thread list is in a standard side nav, but that makes it awkward to render (rendering logic needs to live in the base controller) and gives it some bad beahviors (like autohiding on mobile). Instead, move it into its own view and make it a little more custom. Ref T2421.
Test Plan: {F38098}
Reviewers: btrahan
Reviewed By: btrahan
CC: aran, chad
Maniphest Tasks: T2421
Differential Revision: https://secure.phabricator.com/D5504
Summary:
Currently, `selected_conpherence_id` is actually a PHID. Instead, use ids everywhere.
Also, use replaceState instead of pushState. This means "back" takes you out of conpherence (not back to the last thread you looked at) but I think that's more consistent with user expectation.
Test Plan: Loaded thread list, loaded specific thread.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D5503
Summary: Ref T2421. For mobile, we don't want to select the first thread, since we'll just show a list of threads. Move this behavior to the client and do it when the page is shown on a desktop (or the view is changed to a desktop).
Test Plan: Viewed `/conpherence/`, saw first thread selected. Switched threads.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2421
Differential Revision: https://secure.phabricator.com/D5502
Summary: With the jump panel re-format, lets move to dust on the homepage for depth.
Test Plan: Tested Chrome Mac and PC
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5524
Summary:
Sorry, I'm bad with puns
{F38258}
vs
{F38259}
It was a tough decision. We went with the latter. See chatlog today.
Test Plan: See screens
Reviewers: epriestley, chad, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5522
Summary:
This actually gives it a smooth look across all OS and browsers. Copy-paste from form inputs.
= Motivation (aka Disclaimer) =
I felt kind of... disturbed that it looked different in every browser / OS combination I have at my disposal. The range is from ugly (Chrome on Ubuntu) to pretty (IE9 on Win7 ¬.¬). I give a few examples
- Ubuntu
- Firefox
- Actually looking very nice. Rounded borders and orange border on focus are default from UA style sheet?
- Chrome
- Looks ugly. 2px inset mid-grey border. What you would expect from Win '95
- 1px inset mid-grey border (Win '98 style) + orange webkit outline.
- Windows
- IE9
- Nice blocky text input with black border. Blue border upon hover. Really black border on focus?
- IE10
- Kind of same as IE9, though I had the feeling that it had a deeper black border
- Firefox
- Looks so normal that it is actually boring
- Chrome
- Looks pretty much normal, until focus where you get the webkit outlines. Ugly
No Mac, since I have no Mac. Also no iPhone/iPad. Have Android 4.1/WP8, though I never visited Phabricator with them
Test Plan:
Looked at it in
- Ubuntu
- Mozilla Firefox
- Google Chrome
- Windows 7/8
- IE 9/10
- Opera (Opera?)
- Mozilla Firefox
- Google Chrome
Everything smooth (exceptions in case of no border-radius/box-shadow)
Reviewers: epriestley, btrahan, chad
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5515
Summary: Simplifies the Pontificate button and makes Control + Enter work again.
Test Plan: Submitted the form by clicking, hitting return, hitting control+enter.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D5472
Summary:
Currently, this behavior binds a ton of IDs. This makes the behavior fragile: if it is invoked on a page without all the right elements, some `JX.$()` tends to explode.
Instead, don't assume anything is present on the page. This allows the behavior to be invoked on other pages (like the "New Conpherence" page) or pages without some elements (like some future thread-only mobile view) without creating JS errors.
Test Plan:
Added comments, added comments with files. Viewed "New conphenrece" page.
NOTE: Control+Enter is currently broken, but this diff didn't change that behavior.
Reviewers: btrahan
Reviewed By: btrahan
CC: aran
Differential Revision: https://secure.phabricator.com/D5468
Summary: Adds an action panel on the left side of the workboard.
Test Plan: Tested Fluid and mobile layouts
Reviewers: epriestley, btrahan
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5496
Summary: Fixes T2833. Lock forms and prevent double submit on control + enter.
Test Plan: Mashed control + enter a whole bunch in Maniphest, got one comment instead of several.
Reviewers: btrahan, edward
Reviewed By: btrahan
CC: aran
Maniphest Tasks: T2833
Differential Revision: https://secure.phabricator.com/D5473
Summary:
Also cleans up some stuff like logged out users a bit. This provides a more subtle alternative to {D5485}.
(This is fairly rough, and the icons need to be sprited if we stick with this approach.)
Test Plan:
{F38047}
{F38048}
Reviewers: hach-que, btrahan
Reviewed By: hach-que
CC: aran, chad
Maniphest Tasks: T2857
Differential Revision: https://secure.phabricator.com/D5494
Summary: Adds action items in the footer of workpanels.
Test Plan: UIExamples on Chrome.
Reviewers: epriestley
Reviewed By: epriestley
CC: aran, Korvin
Differential Revision: https://secure.phabricator.com/D5492
