Fixes T6683.
Details
Details
- Reviewers
epriestley chad - Maniphest Tasks
- T6683: provide a keyboard shortcut for expanding hidden changes
- Commits
- Restricted Diffusion Commit
rP9b865f18e845: Transactions - make "show older" area bigger and include keyboard command
clicked the yellow box and it worked! pressed '~' and it worked!
Diff Detail
Diff Detail
- Repository
- rP Phabricator
- Branch
- T6683
- Lint
Lint Passed - Unit
No Test Coverage - Build Status
Buildable 3218 Build 3224: [Placeholder Plan] Wait for 30 Seconds
Event Timeline
Comment Actions
...I figure this gives you another chance to poke holes in this javascript. :D
| webroot/rsrc/js/application/transactions/behavior-show-older-transactions.js | ||
|---|---|---|
| 108 | i need to include this at the top of the file | |
Comment Actions
I'm not totally convinced that the keyboard command is useful / worth-the-cost, but obviously it's like trivial so whatevs.
Various JS stretch goals inline. Nothing looks bad, but I think a few things could be simpler..
Rather than managing JX.Busy manually, I think you can do something similar to queue() in javelin-behavior-workflow:
var routable = workflow.getRoutable()
.setPriority(2000)
.setType('workflow');
JX.Router.getInstance().queue(routable);This replaces workflow.start(), and manages JX.Busy for you. I think that would let you get rid of loading, too.
| webroot/rsrc/js/application/transactions/behavior-show-older-transactions.js | ||
|---|---|---|
| 30–31 | Is this the same as just calling load_older()? | |
| 82–85 | The queue stuff could go here, I think -- then remove the .start() calls. | |
| 93–96 | It might be slightly simpler to pass this .href in on config (like loadURI or something), since the link's href is never actually used/relevant (you can't get anything by clicking the link). It might hypothetically be nice to make the link a real link to the current page with some parameter like ?xactions=all, then have that load everything when building the timeline. Then this feature would still work (just not quite as nicely) if we break the javascript. The javascript URI for ajax loading could be passed through config. | |
| 102–112 | Can you simply remove this? Clicks on the link should also be clicks on the block. | |
Comment Actions
clean up js. only un-taken suggestion is to move the href into config - this href changes to include the proper pagination key as generated by the server, so changing it strikes me as more complex.