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

Fix focus rectangle regression #942

Merged
merged 5 commits into from
Jun 3, 2017
Merged

Fix focus rectangle regression #942

merged 5 commits into from
Jun 3, 2017

Conversation

jasmussen
Copy link
Contributor

This PR fixes a focus rectangle regression I introduced in a previous commit. In doing so, it's also starting work on #904. More work will need to be done, I suspect, and designs tuned, but this PR improves things.

jasmussen added 2 commits May 30, 2017 14:02
Also custom style the focus for editable toolbars.
We are likely going to need to tweak these further as we go, but for now going with the mockups.
@jasmussen jasmussen added the General Interface Parts of the UI which don't fall neatly under other labels. label May 30, 2017
@jasmussen jasmussen self-assigned this May 30, 2017
@jasmussen jasmussen requested a review from ellatrix May 30, 2017 15:00
@aduth
Copy link
Member

aduth commented May 31, 2017

What are testing instructions?

@ellatrix
Copy link
Member

ellatrix commented Jun 1, 2017

@joen Looks great. I'm getting two rectangles for toolbar buttons:

screenshot 2017-06-01 15 00 35

@jasmussen
Copy link
Contributor Author

Ack sorry missed the two comments here.

Testing instructions are tabbing through buttons, and verifying there's a focus rectangle. I'm going to have to remove the one on toolbar buttons, that's a regression, and I'm going to darken the outline a little bit also.

@jasmussen
Copy link
Contributor Author

I've updated this PR and now it should be good to review:

  • Tab through toolbar and sidebar items, verify there's a focus outline.

@ellatrix
Copy link
Member

ellatrix commented Jun 2, 2017

@jasmussen I'm still seeing two lines. Is this intended?

screenshot 2017-06-02 20 31 54

@jasmussen
Copy link
Contributor Author

Argh, can't believe this took so long. Should finally be fixed now.

Copy link
Member

@ellatrix ellatrix left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
General Interface Parts of the UI which don't fall neatly under other labels.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants