8000 Refactor panels.js, messages.js, overlay.js · Issue #261 · fracture91/ignpp · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Refactor panels.js, messages.js, overlay.js #261

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
fracture91 opened this issue Jul 24, 2011 · 2 comments
Open

Refactor panels.js, messages.js, overlay.js #261

fracture91 opened this issue Jul 24, 2011 · 2 comments
Labels
code
Milestone
2.4.0

Comments

@fracture91
Copy link
Owner

The code behind these things is generally horrible "object-based" stuff that would greatly benefit from proper OOP. Here's a preliminary class diagram from yuml.me:

Class diagram

Source:

[View],
[Panel]-[note: No longer read from HTML constructed through DOM instead],
[View]^[Panel],
[Panel]^[ResizablePanel],
[Editor]-[note: Now responsible for setting up pretext based on the type of panel or overlay],
[View]^[Editor],
[Overlay],
[View]^[Overlay],
[Overlay]<->[Editor],
[PostPanel],
[PostPanel]<->[Editor],
[ResizablePanel]^[PostPanel],
[PostPanel]^[ReplyPanel],
[PostPanel]^[TopicPanel],
[PostPanel]^[EditPanel],
[PostPanel]^[PMPanel],
[QuickstatsPanel]
[Panel]^[QuickstatsPanel],
[ThreadPreviewPanel]
[Panel]^[ThreadPreviewPanel],
#
[Model],
[Manager],
[Model]^[Manager],
[Manager]^[PanelManager],
[PanelManager]-<0..n>[Panel],
[PostPanelManager]-<0..n>[PostPanel],
[ThreadPreviewPanelManager]-<0..n>[ThreadPreviewPanel],
[QuickstatsPanelManager]-<0..n>[QuickstatsPanel],
[PanelManager]^[PostPanelManager],
[PanelManager]^[ThreadPreviewPanelManager],
[PanelManager]^[QuickstatsPanelManager],
#
[Messenger],
[Model]^[Messenger],
[Messenger]^[ReplyMessenger],
[Messenger]^[TopicMessenger],
[Messenger]^[EditMessenger],
[Messenger]^[PMMessenger],
[PostPanel]++-[Messenger],
[ReplyPanel]++-[ReplyMessenger],
[TopicPanel]++-[TopicMessenger],
[EditPanel]++-[EditMessenger],
[PMPanel]++-[PMMessenger]

View, Model, and Manager would be something like what Distillery uses.

This should solve some big annoyances:

  • "Infopanels" used by Thread Preview and Quickstats would now share some implementation through the new Panel class, and would probably be a lot less assy.
  • Pretext would be handled by Editor, rather than being spread all over
  • Separate types of panels can handle their own implementation, rather than using stupid switch statements on a type property
  • Separate types of messengers can handle their own implementation, like above
  • PostPanels can use their Messenger as a model to get log messages from
  • PostPanels would no longer read HTML from disk - no disk access on Firefox, should be faster
  • Post metadata (thread ID, etc) can be held by the messenger, can default to stuff that makes sense (current URL)
@fracture91
Copy link
Owner Author

Instead of having the manager classes for views, it may be better to keep an array of all views in view.prototype. When view's constructor is called, push the new instance into the array. Something like:

var Tracked = extend(null, function() {
    this.allTracked.push(this);
},
{
    get all() {
        return this.allTracked;
    },
    allTracked: [],
    remove: function() {
        this.allTracked.splice(this.allTracked.indexOf(this), 1);
    }
});

var View = extend(Tracked, function() {
    Tracked.call(this);
    this.allView.push(this);
},
{
    get all() {
        return this.allView;
    },
    allView: [],
    remove: function() {
        Tracked.prototype.remove.call(this);
        this.allView.splice(this.allView.indexOf(this), 1);
    }
});

var Panel = extend(View, function() {
    View.call(this);
    this.allPanel.push(this);
},
{
    get all() {
        return this.allPanel;
    },
    allPanel: [],
    remove: function() {
        View.prototype.remove.call(this);
        this.allPanel.splice(this.allPanel.indexOf(this), 1);
    }
});

I've tested it, and it works. Should probably make a Tracked.prototype.subclassFactory method or something.

Still need to decide if this would be better or worse overall, though.

@fracture91
Copy link
Owner Author

Yes, I'm going with the self-tracking instances since I spent so much time getting them to work. Very pretty, you just subclass Trackable and then all instances of the subclass are tracked in subclass.prototype.instances. Instances property also holds all instances of subclasses.

Need to update class diagram.

fracture91 added a commit that referenced this issue Jul 27, 2011
@fracture91
Added classes Employee, Observable, Trackable. Tests for Observable a…
5d3eaa5 
…nd Trackable.

Trackable class tracks all instances of itself and subclasses, same for subclasses.
Related to #261.
fracture91 added a commit that referenced this issue Jul 27, 2011
@fracture91
Added MVC classes and tests. Related to #261.
d6641da
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
code
Projects
None yet
Milestone
2.4.0
Development

No branches or pull requests
1 participant
@fracture91
0