Show binding changes #263
Show binding changes #263
Conversation
|
(@rails-bot has picked a reviewer for you, use r? to override) |
| // Change the binding of the console | ||
| /** | ||
| * Change the binding of the console | ||
| * Also updates a div to show that it has been changed. |
There was a problem hiding this comment.
This may change, maybe we won't win a lot by changing the comment here.
There was a problem hiding this comment.
Makes sense, I'll revert the change here
| var notification = document.createElement('div'); | ||
| var _this = this; | ||
|
|
||
| notification.className = "console-notification"; |
There was a problem hiding this comment.
var notification = document.createElement('div');
notification.className = "console-notification";
var _this = this;
postRequest(url, params, function() {Can we keep the flow like this here? The operations are "grouped" this way.
|
|
||
| notification.className = "console-notification"; | ||
| postRequest(url, params, function() { | ||
| notification.innerHTML = "Binding has changed to: " + callback(); |
There was a problem hiding this comment.
I'm thinking whether we call this Context has changed to: because we cannot expect the web console users to know about bindings or the terminology around how code evaluation works in Ruby.
There was a problem hiding this comment.
I initially had it as Context as well but renamed it last minute. I guess I can change it back! I agree it sounds more "intuitive" for people using web-console.
| @@ -3,6 +3,8 @@ | |||
| <div class='console-actions'> | |||
| <div class='close-button button' title='close'>x</div> | |||
| </div> | |||
| <div class='console-inner'></div> | |||
| <div class='console-inner'> | |||
| <div class='console-notifications'></div> | |||
There was a problem hiding this comment.
Do we really need the extra layout here? What's so special about the notifications, they look like a regular console line to me.
There was a problem hiding this comment.
Initially I had the console-notifications set to be updated whenever the binding changes, but it will disappear out of the scrolling window when the commands exceeds the scrollable view.
I have changed it to append before the lastChild of the prompt-box so that it remains in view, but I forgot to remove the unused console-notifications div. It is now removed!
| REPLConsole.prototype.switchBindingTo = function(frameId, callback) { | ||
| var url = this.getSessionUrl('trace'); | ||
| var params = "frame_id=" + encodeURIComponent(frameId); | ||
| postRequest(url, params, callback); | ||
| var notification = document.createElement('div'); | ||
| notification.className = "console-notification"; |
There was a problem hiding this comment.
Do we need to just feed a new input line?
There was a problem hiding this comment.
Currently the default unclassed console text color is black so it won't be visible. I can remove the div but the console text color will need to be changed
…rite(notification)
|
I have refactored the Also introduced a Also made all colour codes consistent. |
|
Reverted the refactoring and added an extra prototype function for writeNotifications instead. |
| @@ -3,6 +3,7 @@ | |||
| <div class='console-actions'> | |||
| <div class='close-button button' title='close'>x</div> | |||
| </div> | |||
| <div class='console-inner'></div> | |||
| <div class='console-inner'> | |||
There was a problem hiding this comment.
We may drop this diff change as well, e.g. leave it as before:
<div class='console-inner'></div>
| @@ -595,13 +595,19 @@ REPLConsole.prototype.writeOutput = function(output) { | |||
| this.inner.appendChild(consoleMessage); | |||
| this.newPromptBox(); | |||
| return consoleMessage; | |||
| }; | |||
| } | |||
There was a problem hiding this comment.
This one too, leave the };, so we don't have this one pop-up in the diff.
|
|
||
| REPLConsole.prototype.writeError = function(output) { | ||
| var consoleMessage = this.writeOutput(output); | ||
| addClass(consoleMessage, "error-message"); | ||
| return consoleMessage; | ||
| }; | ||
| } |
| var consoleMessage = this.writeOutput(output); | ||
| addClass(consoleMessage, "notification-message"); | ||
| return consoleMessage; | ||
| } |
Currently on rails error pages, the change of the binding when clicking on the Trace links is not very visible to users.
This PR will add a notification div that will notify the user the current binding that the user is working with in the console.
It uses a simple callback with the anchor's innerHTML and displays it above the last prompt-box.