Skip to content

ScrollDirection and BlockDirection confusion #151

Closed
Closed
ScrollDirection and BlockDirection confusion#151
#1073 (+2)
Assignees
abarth
Labels
c: API breakBackwards-incompatible API changesc: contributor-productivityTeam-specific productivity, code health, technical debt.frameworkflutter/packages/flutter repository. See also f: labels.

Description

@FlutterIssues
FlutterIssues
opened on Nov 9, 2015

Issue by Hixie
Thursday Aug 27, 2015 at 17:12 GMT
Originally opened as https://github.com/flutter/engine/issues/888

Right now scrollables use ScrollDirection and they're backed by render objects that use BlockDirection. However, that means ScrollDirection.both has no good mapping.

We should clean this up. I think we should probably still have two enums, one with "horizontal" and "vertical" and one with "horizontal", "vertical", and "both", but we should not split it across Block vs Scroll, instead of should split it according to who supports "both" and who doesn't.

Activity

  1. FlutterIssues
    added
    affects: dev experience
    frameworkflutter/packages/flutter repository. See also f: labels.
    on Nov 9, 2015

  2. FlutterIssues commented on Nov 9, 2015

    @FlutterIssues
    FlutterIssues
    on Nov 9, 2015
    Author

    Comment by Hixie
    Tuesday Sep 15, 2015 at 17:58 GMT

    Briefly looked at this with @collinjackson. Scrollable right now asserts that you don't use "both". RenderViewport supports "both", nobody else seems to. We probably want a Widget to wrap RenderViewport with fling gesture support that does both, but other than that we should probably not have any use of the enum with "both".

  3. abarth
    self-assigned this
    on Dec 5, 2015

  4. abarth commented on Dec 5, 2015

    @abarth
    abarth
    on Dec 5, 2015
    Contributor

    Happy to fix this issue, but I'm not clear what we want. Here's a guess:

    • Scrollable should use an enum with just two values (horizontal and vertical)
    • Scrollable and RenderBlock should use the same enum
    • Viewport should use a different enum with three values

    Questions:

    • ScrollDirection is a nice name that makes sense for Scrollable. Should we use that name for RenderBlock too even though RenderBlocks aren't necessarily related to scrolling?
    • What should we call the enum with three values (horizontal, vertical, both) that Viewport uses?

  5. Hixie commented on Dec 5, 2015

    @Hixie
    Hixie
    on Dec 5, 2015
    Contributor

    Axisand MultiAxis?

  6. Hixie
    added
    c: contributor-productivityTeam-specific productivity, code health, technical debt.
    c: API breakBackwards-incompatible API changes
    on Dec 12, 2015
  7. Hixie
    added this to the milestone on Dec 12, 2015
  8. abarth
    removed their assignment
    on Dec 30, 2015
  9. abarth
    added a commit that references this issue on Jan 5, 2016

    Remove ScrollDirection.both

    7aa575b
  10. abarth
    closed this as completedin #1073on Jan 5, 2016

  11. Hixie commented on Jan 5, 2016

    @Hixie
    Hixie
    on Jan 5, 2016
    Contributor

    Should we still merge ScrollDirection and BlockDirection into a single enum?

  12. abarth
    reopened this on Jan 5, 2016

  13. abarth commented on Jan 5, 2016

    @abarth
    abarth
    on Jan 5, 2016
    Contributor

    Sorry, didn't mean to close this issue. Yes, we still need to delete BlockDirection.

  14. 1 remaining item

  15. Hixie commented on Jan 5, 2016

    @Hixie
    Hixie
    on Jan 5, 2016
    Contributor

    Looking at abarth@4246237 (from PR #1074), I really think we should just use "Axis" as the enum name.

  16. abarth commented on Jan 5, 2016

    @abarth
    abarth
    on Jan 5, 2016
    Contributor

    Makes sense.

  17. abarth
    added a commit that references this issue on Jan 7, 2016

    Remove BlockDirection in favor of ScrollDirection

    76dd622
  18. abarth
    closed this as completedin #1114on Jan 7, 2016

  19. Hixie commented on Jan 7, 2016

    @Hixie
    Hixie
    on Jan 7, 2016
    Contributor

    Oh GitHub, you mean well...

  20. Hixie
    reopened this on Jan 7, 2016
  21. abarth
    closed this as completedin #1187on Jan 11, 2016

  22. github-actions commented on Aug 17, 2021

    @github-actions
    github-actionsbot
    on Aug 17, 2021

    This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.

  23. github-actions
    locked as resolved and limited conversation to collaborators on Aug 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

c: API breakBackwards-incompatible API changesc: contributor-productivityTeam-specific productivity, code health, technical debt.frameworkflutter/packages/flutter repository. See also f: labels.

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions