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

NEW: Add first responder management based on content view to CPScrollView #2757

Merged

Conversation

didierkorthoudt
Copy link
Contributor

This will (un)set CPThemeStateFirstResponder on CPScrollView based on the content view.

For now, only CPTableView's can initiate first responder state on a CPScrollView (and only if there's no in-cell editing).

This should be adapted when global focus ring management will be added to Cappuccino.

With this PR, it's easy to add a focus ring around a CPScrollView containing a CPTableView which is the first responder of the window (this will be used by Aristo3).

@mrcarlberg
Copy link
Member

@didierkorthoudt Can you please look into why the test case will fail.

CPScrollViewTest.......
addFailure test=[CPScrollViewTest testNotificationsRegistered]: Notications registered for the scrollView in the notification center are wrong expected:<@[
@"CPScrollerStyleGlobalChangeNotification",
@"_CPWindowDidChangeFirstResponderNotification"
]> but was:<@[
@"CPScrollerStyleGlobalChangeNotification"
]>

@didierkorthoudt
Copy link
Contributor Author

@mrcarlberg Well, I've added a new notification listening in order to implement this new functionality. So I guess that the unit test is not adapted. I check immediately. Sorry for this.

@didierkorthoudt
Copy link
Contributor Author

@mrcarlberg Done.

@mrcarlberg mrcarlberg merged commit 73f02cc into cappuccino:master Aug 21, 2018
@mrcarlberg
Copy link
Member

Merged! Thanks!

@cappbot cappbot added this to the Someday milestone Aug 31, 2018
@cappbot cappbot added the #new label Aug 31, 2018
@cappbot
Copy link

cappbot commented May 8, 2019

Milestone: Someday. Label: #new. What's next? A reviewer should examine this issue.

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

Successfully merging this pull request may close these issues.

None yet

3 participants