Fix the problem that the last flow tab can be deleted #1614
Conversation
|
Hi, thanks for this. Not sure checking the icon path is the safest way to implement this check. I would suggest this logic belongs in |
|
Thank you for pointing it out. Yes, we should have managed the number of tabs in |
editor/js/ui/workspaces.js
Outdated
| @@ -240,18 +241,20 @@ RED.workspaces = (function() { | |||
| } | |||
| }, | |||
| onadd: function(tab) { | |||
| workspaceTabCount = tab.type === "tab"?workspaceTabCount+1:workspaceTabCount; | |||
There was a problem hiding this comment.
From a code style point of view, I'd prefer if this was the easier to read alternative:
if (tab.type === "tab") {
workspaceTabCount++;
}
editor/js/ui/workspaces.js
Outdated
| showWorkspace(); | ||
| } | ||
| }, | ||
| onremove: function(tab) { | ||
| RED.menu.setDisabled("menu-item-workspace-delete",workspace_tabs.count() <= 1); | ||
| if (workspace_tabs.count() === 0) { | ||
| workspaceTabCount = tab.type === "tab"?workspaceTabCount-1:workspaceTabCount; |
There was a problem hiding this comment.
Same as the comment on line 244.
|
Thank you for pointing it out. I've pushed another commit following your suggestion. |
|
Great thanks! |
Before you hit that Submit button....
Please read our contribution guidelines
before submitting a pull-request.
Types of changes
What types of changes does your code introduce?
Put an
xin the boxes that applyIf you want to raise a pull-request with a new feature, or a refactoring
of existing code, it may well get rejected if it hasn't been discussed on
the mailing list or
slack team first.
Proposed changes
see #1613.
I checked all callers of
workspace_tabs.count(). It looks that all of them expect that the count does not include subflow tabs.Checklist
Put an
xin the boxes that applygruntto verify the unit tests pass