Conversation
chrismohr
requested review from
cainhimself,
dan5382,
richardtagger,
swese44,
unindented and
vanschyndel
as code owners
| .y-icon__isBlock { | ||
| display: block; | ||
| position: static; | ||
| margin-top: 0.2rem; |
There was a problem hiding this comment.
I was torn about adding the margin here. Unless I'm missing something, the alternative was to add wrapping blocks with push=2, to many uses of Icon.
There was a problem hiding this comment.
There was a problem hiding this comment.
Sounds good. I'll revert the changes related to #327.
| MenuButton with default Icon, and all applicable children properties | ||
|
|
||
| ```js { "props": { "data-description": "with default icon and all menu item types" } } | ||
| ```js { "props": { "data-description": "with default icon and all menu item types", "data-action-states": "[{\"action\":\"none\"},{\"action\":\"click\",\"selector\":\".y-menu-button\"}]" } } |
There was a problem hiding this comment.
can you do all of these interactive diffs in a separate PR? the change required to fix #327 worries me a bit, it would be nice to have the screenshots in place for the current code first. thanks!
chrismohr
force-pushed
the
action_link_styles
branch
from
ae87248 to
e7ea13e
Compare
| <Icon size={IconSize.MEDIUM} block={true} className="y-actionLink--icon" /> | ||
| <Block push={2}> | ||
| <Icon size={IconSize.MEDIUM} block={true} /> | ||
| </Block> |
There was a problem hiding this comment.
I'm probably missing something here, I used a Block instead of
.y-actionLink--icon {
margin-top: 0.4rem;
/* Remove when this is addressed: https://github.com/Microsoft/YamUI/issues/327 */
position: static;
top: 0;
}There was a problem hiding this comment.
i think i left it as CSS because it was clear it needed to be updated when the issue was resolved, and lighter-weight solution. if you're going to move it to markup could you keep the comment with it?
There was a problem hiding this comment.
I tried css, but switched back again, because the style was losing out to the styles on .y-icon. I presume this is because the merge-styles are being loaded before the css styles. I've added the comment above the render function.
2 participants
Uh oh!
There was an error while loading. Please reload this page.