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

code interface cleanup #133

Open
guruofgentoo opened this issue Oct 14, 2020 · 0 comments
Open

code interface cleanup #133

guruofgentoo opened this issue Oct 14, 2020 · 0 comments

Comments

@guruofgentoo
Copy link
Member

guruofgentoo commented Oct 14, 2020

While working through documentation, I'm tracking things that seem inconsistent/wrong/needs-refactored. We'll need to triage these into near/long term updates. A few of these may be represented in other issues.

Grid:

  • identifier/ident
    • Why not have a single identifier attribute and just set it at instance time?
  • features inconsistently named
    • feature flags
      • sorter_on, pager_on, session_on, etc.
      • enable_search, hide_controls_box, subtotals
  • renderer attributes are set directly on the grid. Seems like they should be scoped to a renderers area.
  • build method
    • not a great name, as it doesn't really build anything. This method seems to be a shortcut to running a before-query hook and producing attribute errors outside of a Jinja context, where they'd be hidden.
    • maybe init_request?
    • also not sure on the intent of the before-query hook. Perhaps it is to have a spot for modifying filters before the query is constructed. Doesn't appear to be used in any of our projects.
  • _apply_filtering and _apply_paging should use the grid's set methods
    • otherwise, if ops are done to retrieve records prior to applying args, cached values will persist
    • much of the logic in paging could be moved to set_paging
  • column init should set a key if it will be None
  • add a set_column_order method that takes a list of keys and sets up the grid with the given column order

Column:

  • Columns need a way to be added at instance time, like with an add_column method
    • helpful to set up the grid at run time, since in some cases app context is required
    • current solution is grid "factory" methods that pass the grid class back, but that seems overkill for the problem
  • NumericColumn has accounting format overrides that seem unintuitive
    • should have an AccountingColumn that encapsulates these

Filter:

  • OptionsEnumFilter requires enum_type to be passed as an arg. Should be able to set it as a class attribute on a subclass instead.
  • set should make value1 optional as well. Many operators do not have inputs.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant