Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Conversation

@tvolkert
Copy link
Contributor

@tvolkert tvolkert commented Jun 9, 2020

Description

This adds engine support for the resize up-down and resize left-right cursors, which enable
callers to build a splitter widget.

Checklist

  • I read the [contributor guide] and followed the process outlined there for submitting PRs.
  • I signed the [CLA].
  • I read and followed the [C++, Objective-C, Java style guides] for the engine.
  • I read the [tree hygiene] wiki page, which explains my responsibilities.
  • I updated/added relevant documentation.
  • All existing and new tests are passing.
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

@fluttergithubbot
Copy link
Contributor

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

@auto-assign auto-assign bot requested a review from chinmaygarde June 9, 2020 02:44
@chinmaygarde chinmaygarde removed their request for review June 9, 2020 20:15
@dkwingsmt
Copy link
Contributor

The code looks good.
As for naming, do we use up/down in Flutter at all? I thought we've been sticking to top/bottom. Also if we use up/down, then should we name the resize-nw as UpLeft or UpperLeft?

@tvolkert
Copy link
Contributor Author

tvolkert commented Jun 9, 2020

Do we support the resize-nw cursor right now?

The top/bottom usages I've seen refer to regions, whereas this refers to directionality. Is there a case where we use top/bottom for directionality?

@dkwingsmt
Copy link
Contributor

dkwingsmt commented Jun 10, 2020

Do we support the resize-nw cursor right now?

Not yet, but only because of low priority, the same as these two cursors.

The top/bottom usages I've seen refer to regions, whereas this refers to directionality. Is there a case where we use top/bottom for directionality?

You're right. So what name do you think they would be (as well as diagnal cursors such as resize-nwse)? Just want to plan everything ahead.

@stuartmorgan-g
Copy link
Contributor

Do we support the resize-nw cursor right now?

Not yet, but only because of low priority, the same as these two cursors.

FYI, diagonal resize aren't system cursors on macOS, so if we want to support them it'll require custom embedding support.

@tvolkert
Copy link
Contributor Author

Maybe horizontalDoubleArrow and verticalDoubleArrow would allow for expansion room (if we decide to do the custom embedder work)?

@dkwingsmt
Copy link
Contributor

dkwingsmt commented Jun 10, 2020

There are lots of cursors that aren't shared between platforms. The plan is to make them fall back to other cursors, such as the basic cursor, so that the superset of all cursors is available to the framework by all platforms, although some don't work in the best way. In other words, having diagnal resize cursors in the framework doesn't mean they will be supported by all platforms, but only that they will be supported by some platforms.

@tvolkert tvolkert changed the title Add support for resizeLeftRight and resizeUpDown cursors Add support for horizontalDoubleArrow and verticalDoubleArrow cursors Jun 10, 2020
@tvolkert tvolkert merged commit 7e6c856 into flutter:master Jun 10, 2020
@tvolkert tvolkert deleted the cursor branch June 10, 2020 14:17
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 10, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 10, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 10, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 10, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 10, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 10, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 11, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 11, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants