Skip to content
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.

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

[feature request] add a context object #634

Closed
aledeg opened this issue Sep 24, 2014 · 13 comments
Closed

[feature request] add a context object #634

aledeg opened this issue Sep 24, 2014 · 13 comments

Comments

@aledeg
Copy link
Member

aledeg commented Sep 24, 2014

See #632 for the context of the discussion.

It would be nice to have a context object that handle the current state of the application and the current configuration for the user.
This way we can have a more efficient way to modify the layout and query the database.

At the moment, here is the list of the thing I can think of:

  • compute the current state to display depending of the user configuration and selection
  • simplify templates by using context methods instead of complex tests
  • simplify queries by using context methods instead of complex tests
@marienfressinaud
Copy link
Member

I marked this ticket as critical since it will be necessary for big changes like plugin system and useful to refactor some parts of the source code.

@marienfressinaud marienfressinaud mentioned this issue Oct 5, 2014
8 tasks
marienfressinaud added a commit that referenced this issue Oct 5, 2014
Introduce kind of context objectin JavaScript

See #634
See #655
@marienfressinaud
Copy link
Member

I'm working on the Context object but I'm not sure of what it should do:

  • Return which state is enabled (e.g. $this->context->stateEnabled(FreshRSS_Entry::STATE_READ) returns true if state read is enabled)
  • Return the next get value (useful in entryContoller::readAction() so we can remove code in nav_menu.phtml)
  • Aaaand… what else? ^^' I would like to simplify the indexController() but I'm not sure Context object can help… what do you think?

@aledeg
Copy link
Member Author

aledeg commented Oct 20, 2014

For me, it should hold the configuration and the current state. It should also make all the calculation regarding the state, the next/previous feed/category and maybe other things.
For instance, I can see methods like getNextFeed, getNextCategory, isCurrentStatusRead.

@aledeg
Copy link
Member Author

aledeg commented Oct 20, 2014

Maybe we can start with a simple object and use it to simplify the code in the controllers, views and DAOs. If something comes up, we can add it later.

@marienfressinaud
Copy link
Member

Ok, fine, I begins something :)

marienfressinaud added a commit that referenced this issue Oct 20, 2014
@marienfressinaud
Copy link
Member

My last commit gives an idea of how to use context object. Here is my plan:

  1. Replace all references to different $conf variables by FreshRSS_Context::$conf
  2. Replace output parameters by different actions: one action for normal view, another for global view and a last one for RSS output. I think reader view should be supported by CSS only (I'm not decided how to handle this view).
  3. Previous point will ask to refactor indexController. It will be time to use furthermore the Context object I think (maybe by storing useful parameters such as search, order, nb, etc.?)
  4. Then, nav_menu.phtml seems to need being polished (use of Context object again).
  5. Finally, check different other view files if Context object can be used.

marienfressinaud referenced this issue Oct 20, 2014
- Replace $this->view->conf in controllers
- Replace $this->conf in views
marienfressinaud added a commit that referenced this issue Oct 21, 2014
Global view has been moved to a different action (all is not working)

See #634
and #655
marienfressinaud added a commit that referenced this issue Oct 21, 2014
- Seperate normal, global and rss outputs in dedicated actions (NOT WORKING YET!)
- Rewrite aside_flux and nav_menu to use Context object
- Improve Context object

See #634
@marienfressinaud
Copy link
Member

I have pushed my work in a dedicated branch. IT'S NOT WORKING ANYMORE. There is still a lot of work to finish but I'm quite satisfied of what I have done so far :)

marienfressinaud added a commit that referenced this issue Oct 22, 2014
@marienfressinaud
Copy link
Member

I have moved a lot of code from indexController to Context object, please have a look to the source code. I have to add comments and finish to fix bugs and missing features.

I hope the code is clearer now: indexController does 186 lines now and 500 before!

marienfressinaud added a commit that referenced this issue Oct 22, 2014
- Context object raises correct Exception if get is invalid
- RSS feed is well-indicated on the home page
- State is better calculated
- Add some comments

See #634
marienfressinaud added a commit that referenced this issue Oct 22, 2014
- Seperate view mode from default state in conf
- Load read articles if no unread articles only if view is adaptive

See #634
marienfressinaud added a commit that referenced this issue Oct 22, 2014
marienfressinaud added a commit that referenced this issue Oct 22, 2014
marienfressinaud added a commit that referenced this issue Oct 22, 2014
marienfressinaud added a commit that referenced this issue Oct 22, 2014
@marienfressinaud
Copy link
Member

I have finished the Context object and fixed some bugs, IndexController has been refactored (see #655) and nav_menu.phtml does not handle the `nextGet`` calculation anymore. I don't merge the code for the moment because I have changed the feed aside structure (something I wanted to do since a long time) but it's not finished yet!

marienfressinaud added a commit that referenced this issue Oct 22, 2014
marienfressinaud added a commit that referenced this issue Oct 23, 2014
marienfressinaud added a commit that referenced this issue Oct 23, 2014
marienfressinaud added a commit that referenced this issue Oct 23, 2014
marienfressinaud added a commit that referenced this issue Oct 23, 2014
@aledeg
Copy link
Member Author

aledeg commented Oct 24, 2014

I just thought about something to add in the context object. We need to store the query parameters and have methods to get those values easily. That would be useful for filtering the result, counting the number of results of a query (see #546 and #611), mark as read a selection (see #608 and #609), and improve the filter options (see #610).

marienfressinaud added a commit that referenced this issue Oct 24, 2014
marienfressinaud added a commit that referenced this issue Oct 24, 2014
marienfressinaud added a commit that referenced this issue Oct 24, 2014
marienfressinaud added a commit that referenced this issue Oct 24, 2014
marienfressinaud added a commit that referenced this issue Oct 24, 2014
@marienfressinaud
Copy link
Member

Do you mean we need to store parameters such as search, nb, order, etc.? Because it's already the case ;)

@aledeg
Copy link
Member Author

aledeg commented Oct 24, 2014

That's what I meant. So 👍
I haven't check the code yet since I am pretty busy at the moment.

@marienfressinaud
Copy link
Member

No problem, we all have our constraints ;)

I have finished my work on this branch so I merge it into dev

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants