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

add .ms-persona.is-selected #327

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
4 participants
@SteenMolberg
Copy link

SteenMolberg commented Feb 7, 2016

Currently doing the OrgChart directive for ngOfficeUiFabric and it would be nice to able to select a person or persons in the OrgChart just like in a tablerow.

@msftclas

This comment has been minimized.

Copy link

msftclas commented Feb 7, 2016

Hi @SteenMolberg, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!

This seems like a small (but important) contribution, so no Contribution License Agreement is required at this point. Real humans will now evaluate your PR.

TTYL, MSBOT;

@mikewheaton

This comment has been minimized.

Copy link
Collaborator

mikewheaton commented Feb 9, 2016

Thanks for submitting this PR, @SteenMolberg. We just need to check in with our design team and have a quick discussion about whether the Persona should be interactive, or if it's just a display template to be used within other controls.

@mikewheaton

This comment has been minimized.

Copy link
Collaborator

mikewheaton commented Feb 10, 2016

@SteenMolberg: Could you send us a screenshot of the OrgChart with a person selected? We just want to see what this looks like in context.

@SteenMolberg

This comment has been minimized.

Copy link
Author

SteenMolberg commented Feb 10, 2016

Below screenshots...

Image1: Items not selectable. Please not the presence icon placement compared to image2
Image2: 2 items selected and mouse hovers over a third item.

officeuifabric1
officeuifabric2

@ericthompson

This comment has been minimized.

Copy link
Contributor

ericthompson commented Feb 11, 2016

Thanks, @SteenMolberg! We're passing this along and we'll get back to you as soon as we can.

@mikewheaton

This comment has been minimized.

Copy link
Collaborator

mikewheaton commented Mar 8, 2016

We've discussed this with the design team and they agree that Persona should have a selected state. We just need to review the official designs to be sure this is in line before we can merge it.

@SteenMolberg

This comment has been minimized.

Copy link
Author

SteenMolberg commented Mar 8, 2016

thx for info Mike

display: table;
line-height: 1;
position: relative;
@include ms-font-m;

This comment has been minimized.

@mikewheaton

mikewheaton Mar 28, 2016

Collaborator

Looks like the indentation has changed on these lines. Could you revert these lines to how they originally were?


// Persone can be selected.
&.is-selected {
background-color: $ms-color-themeLight;

This comment has been minimized.

@mikewheaton

mikewheaton Mar 28, 2016

Collaborator

I found some design documentation that shows we should be using $ms-color-themeLighter. Could you please change it?

@mikewheaton

This comment has been minimized.

Copy link
Collaborator

mikewheaton commented Mar 28, 2016

Apologies for the delays here, @SteenMolberg. I've added a couple comments – could you address these and add a new commit to your pull request?

@mikewheaton

This comment has been minimized.

Copy link
Collaborator

mikewheaton commented May 28, 2016

Adding the needs design flag here so that we can verify the selected state.

@mikewheaton

This comment has been minimized.

Copy link
Collaborator

mikewheaton commented Jun 1, 2016

I've opened a new issue to request official designs for these states. We appreciate this PR but it seems to have been abandoned, and with the recent changes for Fabric 3.0 coming in it no longer merges without conflicts.

@mikewheaton mikewheaton closed this Jun 1, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment