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

Limit the tooltip width of Ports #9813

Merged
merged 1 commit into from Jun 25, 2019

Conversation

ZiyunShang
Copy link
Contributor

Purpose

Limit the tooltip width of Ports so that the bubble will not be very long.
LongToolTipBubble
ShortToolTipBubble

Declarations

Check these if you believe they are true

  • The code base is in a better state after this PR
  • Is documented according to the standards
  • The level of testing this PR includes is appropriate
  • User facing strings, if any, are extracted into *.resx files
  • All tests pass using the self-service CI.
  • Snapshot of UI changes, if any.
  • Changes to the API follow Semantic Versioning, and are documented in the API Changes document.

Reviewers

@QilongTang @mjkkirschner

FYIs

@AndyDu1985

@@ -67,7 +67,7 @@
<Grid.ToolTip>
<dynui:DynamoToolTip AttachmentSide="{Binding Path=PortType, Converter={StaticResource PortToAttachmentConverter}}" Style="{DynamicResource ResourceKey=SLightToolTip}">
<Grid>
<TextBlock Text="{Binding Path=ToolTipContent}"></TextBlock>
<TextBlock MaxWidth="320" TextWrapping="Wrap" Text="{Binding Path=ToolTipContent}"></TextBlock>
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you elaborate here how did you define 320? If possible can you move the number to be a const number property?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I found that the node Rectangle ToolTip was set width as 320. (

<TextBlock MaxWidth="320"
)
So I just keep the same as it.

@QilongTang
Copy link
Contributor

I am fine with us fixing it this way for now, LGTM as long as you put TODO comment for our team to move those magic numbers to resource strings

@ZiyunShang
Copy link
Contributor Author

ZiyunShang commented Jun 25, 2019

OK @QilongTang @mjkkirschner , I have create an issue on DYN Jira(https://jira.autodesk.com/browse/DYN-2000), you can take a look at it if you have time ; )

@QilongTang
Copy link
Contributor

@ZiyunShang Would you provide Jira link here or @ me and @mjkkirschner in it, otherwise we will not get a notification? Thanks!

@ZiyunShang
Copy link
Contributor Author

@QilongTang Ok, I have add the link here.

@QilongTang QilongTang added the LGTM Looks good to me label Jun 25, 2019
@ZiyunShang
Copy link
Contributor Author

@QilongTang @mjkkirschner could you help merge this?

@QilongTang QilongTang merged commit 6049ec9 into DynamoDS:master Jun 25, 2019
mjkkirschner pushed a commit that referenced this pull request Aug 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
LGTM Looks good to me
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants