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

[GH-1057] Highlighting on hover for property values in the card dialog #1065

Merged

Conversation

kamre
Copy link
Contributor

@kamre kamre commented Aug 22, 2021

Summary

Highlighting on hover for property values in the card dialog:

  • only values that are not readonly highlighted (similar to Notion)
  • all fields for values have min-width equal to 150px (same as for property name)
  • text fields like Email/URL/... are expanded when needed
  • width of property name buttons is the same
  • LastModifiedAt/LastModifiedBy/CreatedAt are considered readonly

Here is how it looks:
HighlightOnHover

Ticket Link

Fixes #1057

@asaadmahmood please review

  - highlight on hover property values that are not readonly
  - make property name buttons the same width
  - set `min-width: 150px` for property values
  - add `readonly` class for LastModifiedAt/LastModifiedBy/CreatedAt
  - input width computation relies on `useLayoutEffect` and `getComputedStyle`
  - enable automatic expand for editable fields in `PropertyValueElement`
  - enable automatic expand for editable inside `URLProperty`
  - fix for tooltip display in `KanbanCard`
@kamre kamre requested a review from a team as a code owner August 22, 2021 14:41
@kamre kamre requested review from harshilsharma63 and chenilim and removed request for a team August 22, 2021 14:41
@mattermod
Copy link
Contributor

Hello @kamre,

Thanks for your pull request! A Core Committer will review your pull request soon. For code contributions, you can learn more about the review process here.

}
}
}


Copy link
Contributor Author

Choose a reason for hiding this comment

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

This file was taken from #982

const input = elementRef.current
const computed = getComputedStyle(input)
input.style.width = 'auto'
input.style.width = `${input.scrollWidth + borderWidth(computed) + 1}px`
Copy link
Contributor Author

Choose a reason for hiding this comment

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

+ 1 here is required for workaround this Chrome issue.

display: flex;

.octo-propertyvalue {
white-space: none;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wrong value for white-space property.

@harshilsharma63
Copy link
Member

Works fine. @asaadmahmood can you run this and verify the UI/UX changes?

@kamre
Copy link
Contributor Author

kamre commented Aug 23, 2021

@asaadmahmood by the way changes here somewhat overlap with yours in #1047

@asaadmahmood
Copy link
Contributor

@harshilsharma63 @kamre Will have a look at this today.

@asaadmahmood asaadmahmood self-requested a review August 23, 2021 13:54
Copy link
Contributor

@asaadmahmood asaadmahmood left a comment

Choose a reason for hiding this comment

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

Looks good to me, pushed a change.

@mattermod
Copy link
Contributor

This PR has been automatically labelled "stale" because it hasn't had recent activity.
A core team member will check in on the status of the PR to help with questions.
Thank you for your contribution!

/cc @jasonblais @jfrerich @emilyacook

@kamre
Copy link
Contributor Author

kamre commented Sep 13, 2021

@chenilim do we need any additional changes for this pull request?

@chenilim chenilim merged commit 50470ef into mattermost-community:main Sep 25, 2021
@chenilim
Copy link
Contributor

Sorry for the delay @kamre! In general, we just need one dev review to merge. We'll tighten up our processes.

Also, I manually merged with main, so please LMK if I missed anything. Thanks again for your contributions! These really up the basic experience of Focalboard!

@kamre kamre deleted the gh-1057-highlight-property-value branch September 25, 2021 07:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature Idea: Property value controls should highlight on hover in the card editor
5 participants