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 - 774] Add tooltip for jira ticket links #835

Closed
wants to merge 50 commits into from

Conversation

sibasankarnayak
Copy link
Contributor

implemented tooltip to show Ticket name, Status, Type, First few characters from the description

ticket here
Fixes #774

@mattermod
Copy link
Contributor

Hello @sibasankarnayak,

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.

@sibasankarnayak
Copy link
Contributor Author

@hanzei @mickmister
this is the screenshot for tooltip , please share your feedback

image

@hanzei hanzei added 1: UX Review Requires review by a UX Designer 2: Dev Review Requires review by a core committer 3: QA Review Requires review by a QA tester labels Feb 10, 2022
@hanzei hanzei added this to the v3.2.0 milestone Feb 10, 2022
@hanzei hanzei requested a review from Rina-dsg February 10, 2022 12:31
@mickmister
Copy link
Contributor

@sibasankarnayak Can you please convert this PR to use typescript? I would like that to be done before I review this. Please let me know if you have any questions about this

@sibasankarnayak
Copy link
Contributor Author

@sibasankarnayak Can you please convert this PR to use typescript? I would like that to be done before I review this. Please let me know if you have any questions about this

Sure @mickmister

@sibasankarnayak
Copy link
Contributor Author

@sibasankarnayak @matthewbirtch, we are using an external library for that particular reason called Tippy. The tour tip component, which we have used, can be found here https://github.com/mattermost/mattermost-webapp/tree/master/components/widgets/tour_tip. @sibasankarnayak you can use the same if that can resolve the issue, but please make sure of using the same version. Please let me know if you need any help.

@matthewbirtch as suggested by @AshishDhama to use a npm package tippyjs - V4.2.6 which was used by mattermost-webapp which is using npm 7
jira plugin is using npm 6 so we are facing problem ,
can we skip this arrow design or might need to wait thill jira started using npm 7 which might take long.

cc @mickmister

@mickmister
Copy link
Contributor

you can use the same if that can resolve the issue, but please make sure of using the same version

@AshishDhama Just want to note that if we need to use the same version of a node module, we would want to first export this to plugins in https://github.com/mattermost/mattermost-webapp/blob/master/plugins/export.js, so we ensure the same version will always be used.

@sibasankarnayak Can you try implementing this by exposing the library via the webapp's exports? We wouldn't get type completion in the plugin's source code without also installing the package in the Jira plugin's project, but the package will be available to use at runtime via the window object.

@AshishDhama
Copy link

@mickmister Thanks will add it there.

@sibasankarnayak sibasankarnayak requested review from mickmister and removed request for mickmister April 21, 2022 14:48
@mickmister
Copy link
Contributor

mickmister commented Apr 25, 2022

@sibasankarnayak Have you created the PR to add Tippy to mattermost-webapp's exports? As I said, we need that to be done as soon as possible to minimize the change in min_server_version in the plugin

@sibasankarnayak
Copy link
Contributor Author

@sibasankarnayak Have you created the PR to add Tippy to mattermost-webapp's exports? As I said, we need that to be done as soon as possible to minimize the change in min_server_version in the plugin

created the PR mattermost/mattermost-webapp#10235

};

return (
<Tippy arrow={true} >

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@AshishDhama for providing reference i need to give a reference to hyperlink but as this is plugin we register this tooltip component on hover , i am not sure if we can add something like as you mentioned in this case.

let me know if we can or you can suggest the changes in PR that will help a lot.

@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!

@raghavaggarwal2308
Copy link
Contributor

@mickmister @AshishDhama For using Tippy or any other tooltip library we need a reference to the actual component which will act as an anchor to the tooltip and in our case the component which needs to be an anchor is present on the Mattermost webapp code so it would not be possible to refer that component directly from the plugin.
However, this can be achieved using some UI hacks but we don't think that would be feasible for rendering the arrow.
However, we have everything similar to the Figma except that arrow which is perfectly fine.
Even the tooltip present in the GitHub plugin doesn't contain the arrow as well and uses Mattermost's default tooltip component.
So, we suggest skipping the arrow and just fixing the typescript errors in this PR, please let us know your thoughts if it's different

Screenshot from 2022-10-25 22-00-10

@mickmister
Copy link
Contributor

For using Tippy or any other tooltip library we need a reference to the actual component

@raghavaggarwal2308 There's an open PR that exposes the component to plugins to use as window.Tippy
mattermost/mattermost-webapp#10235

So, we suggest skipping the arrow and just fixing the typescript errors in this PR, please let us know your thoughts if it's different

I'm fine with skipping the arrow if necessary

@raghavaggarwal2308
Copy link
Contributor

@mickmister Since the PR #10235 has not merged yet. We suggest merging this PR with the typescript errors fixed, without the arrow and we can revisit that change once PR #10235 is merged.

Created issue #886 for the same.

@hanzei hanzei closed this Feb 21, 2023
@hanzei hanzei removed 2: Dev Review Requires review by a core committer Lifecycle/1:stale 3: QA Review Requires review by a QA tester 1: UX Review Requires review by a UX Designer labels Feb 21, 2023
@hanzei hanzei removed this from the v3.3.0 milestone Feb 21, 2023
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.

Add tooltip for jira ticket links