feat: display openrank information in developer information hover card

[Tenth-crew]

Brief Information

This pull request is in the type of (more info about types):

• build
• ci
• docs
• feat
• fix
• perf
• refactor
• test

Related issues: [Task 6 Display extra information from OpenDigger in the developer information hover card]#810

Details

Checklist

• I have read the CONTRIBUTING doc
• I have signed the CLA

Others

I'm sorry that a previous commit was lost, leaving only the run prettier commit

I haven't completed the task yet. I want to ask through this draft PR why my code doesn't work and I can't find the error.

[wxharry]

Hi Tenth, thanks for your work! It is good to have a draft PR to discuss.

To answer your question, why it is not working. I think it is because the element with avatar does not have id js-global-screen-reader-notice'. I am not sure what that element is. I searched that id in source code and it was at the bottom of the page, not the avatar. You can add a console.log('target:', target.id);` to see the ids of the element your mouse is hovering.

Also, the use of document.addEventListener('mouseover', is not a good way to add a feature, it could slow down the system drastically. One way I can think of is to use await elementReady to detect it when the element within the popover is ready, and then insert the info we want. Please Let me know if there is a specific reason we have to use event listener here.

Feel free to reach out if you have further questions. I would love to help. 😸

[Tenth-crew]

Thanks for you answer! Why i use document.addEventListener('mouseover') because I am not familiar with the event listening code. I will try to modify it with await elementReady。And In the code, I use id js-global-screen-reader-notice to find the developer information when the mouse hovers based on the following findings in the element

[Tenth-crew]

When mouse hover the avator the target do not return js-global-screen-reader-notice, i will try use other id or class to find the right element

[Tenth-crew]

Now I set the target element of the mouse hover to an element with the data-hovercard-type attribute, but I don't know why the target does not respond when the mouse hovers over the avatar.

The mouse hover I am listening for now is to detect whether there is a data-hovercard-type when the mouse hovers over the img element or to detect whether the nearest parent element of the img element has a data-hovercard-type. Because in different scenarios, the location of data-hovercard-type is different.

[wxharry]

Hi, sorry for the delay reply. Here are my advice on your questions. This is not a solution, feel free to discuss more. Yes, you are right. Looking for the [data-hovercard-type="user"] attribute is better than looking for the id

Code first:

  document.querySelectorAll('[data-hovercard-type="user"]').forEach((element) => {
    element.addEventListener('mouseover', async () => {
      console.log('mouseover', element)
      const $popoverContainer = 'body > div.logged-in.env-production.page-responsive > div.Popover.js-hovercard-content.position-absolute > div.Popover-message.Popover-message--large.Box.color-shadow-large.Popover-message--right-bottom > div > div'
      const popover = await elementReady($popoverContainer, {stopOnDomReady: false})
      console.log("popover", popover)
      const y = $($popoverContainer).find('a').find('img')
      y.attr('src', 'https://avatars.githubusercontent.com/u/39271899?v=4')
    });
  })

Some changes:

1. Instead of adding event listener to document, I added it to elements we want them to listen to
2. I use elementReady to select the popover container
3. We might need to wait til the hovercard displays

Things need to look into:

1. Adding event listener to the elements not working for all needed elements, not sure why
2. I set {stopOnDomReady: false} to make it work, but not sure this is a good way, need to look into.
3. In my example, I replaced src, you need to look for the element we want and add to it.

[Tenth-crew]

Thanks! i will try it later

[Tenth-crew]

Thanks for your reply again.Through your code, I have a deeper understanding of the event monitoring method. Now I can import OpenRank information, but I found that when the mouse hovers over the avatar for the first few hundred milliseconds, OpenRank information appears in js-global-screen-reader-notice, but is then overwritten by the user data generated by the GitHub script. I tried to monitor this overlay, and the result was that my monitoring code kept fighting against the GitHub script, making the hovercard unable to display normally.

That is, when the floating card appears, it is based on the user data obtained by the GitHub script, and cannot display the OpenRank information we introduced. I don't know much about GitHub scripts. Is there any way to make our information display correctly instead of being constantly overwritten? If so, I'd like to try it!

If there is no feasible method, I thought of another solution, which is to create a new container to present the openrank information instead of using js-global-screen-reader-notice

[wxharry]

If there is no feasible method, I thought of another solution, which is to create a new container to present the openrank information instead of using js-global-screen-reader-notice

I am not sure what that is but I don't think it is a good way to change the inner content of that element becuase it is just a string and we don't know how Github parse it. I would recommend inserting data to the hovercard direclty when it pops up. We can improve the performance somehow by caching the user info to prevent constantly requesting data. Let me know what you think.

[Tenth-crew]

If there is no feasible method, I thought of another solution, which is to create a new container to present the openrank information instead of using js-global-screen-reader-notice

I am not sure what that is but I don't think it is a good way to change the inner content of that element becuase it is just a string and we don't know how Github parse it. I would recommend inserting data to the hovercard direclty when it pops up. We can improve the performance somehow by caching the user info to prevent constantly requesting data. Let me know what you think.

From my observation, <div id=‘js-global-screen-reader-notice’></div> is where GitHub dynamically obtains and displays user information. My previous method was to insert the openrank information before the user information, as shown in my previous reply, it has been successfully inserted. However, GitHub will re-obtain the user information and then overwrite the information I have inserted. I also tried to monitor the coverage of<div id=‘js-global-screen-reader-notice’></div>and then reinsert my openrank information, but this resulted in constant overwriting between GitHub and the openrank information. So now I am trying to try another way to display the openrank information, and I will give a schematic diagram later.

[Tenth-crew]

I want to try this method. Do you think it's okay? It's not perfect yet. If you think it's okay, I'll continue.

[Tenth-crew]

I want to try this method. Do you think it's okay? It's not perfect yet. If you think it's okay, I'll continue.

If this solution is ok, I am ready to review it.

[wxharry]

I am not sure this is what we want. From the description, I was expecting something like this:

2024-07-16.19.36.32-1.mov

Let me know if you have any concerns or troubles. We can also schedule a meeting to discuss if you want.

[Tenth-crew]

I think what you showed in your video is the expected result of the original issue. How did you do it?

[wxharry]

I think what you showed in your video is the expected result of the original issue. How did you do it?

I followed the code in this comment #832 (comment)

[Tenth-crew]

Based on the code you showed in #832, here is my code, but my result is as shown in the video, I added the content to the end of the card information, but it was overwritten afterwards

document.querySelectorAll('[data-hovercard-type="user"]').forEach((element) => {
    element.addEventListener('mouseover', async () => {
      console.log('mouseover', element);

      // 获取开发者名称
      const developerName = getDeveloperName(element as HTMLElement);
      if (!developerName) {
        console.error('Developer name not found');
        return;
      }

      console.log('developerName', developerName);

      // 获取悬浮卡片容器
      const $popoverContainer = 'body > div.sr-only.mt-n1';
      const popover = await elementReady($popoverContainer, { stopOnDomReady: false });
      console.log('popover', popover);

      // 获取开发者的openrank信息
      const openrank = await getData(developerName);
      if (openrank === null) {
        console.error('Rank data not found');
        return;
      }

      // 将 OpenRank 信息作为红色文本直接插入到悬浮卡片容器内容的前面
      if (popover) {
        popover.innerHTML = popover.innerHTML + ` OpenRank ${openrank} `;
      }
    });
  });

vedio1.mp4

[wxharry]

I used body > div.logged-in.env-production.page-responsive > div.Popover.js-hovercard-content.position-absolute > div.Popover-message.Popover-message--large.Box.color-shadow-large.Popover-message--right-bottom > div > div which is the actual hovercard, instead of the <div id=‘js-global-screen-reader-notice’>. After we get the hovercard, we can insert anything we want.

[Tenth-crew]

I used body > div.logged-in.env-production.page-responsive > div.Popover.js-hovercard-content.position-absolute > div.Popover-message.Popover-message--large.Box.color-shadow-large.Popover-message--right-bottom > div > div which is the actual hovercard, instead of the <div id=‘js-global-screen-reader-notice’>. After we get the hovercard, we can insert anything we want.

Thanks, I'll try it soon!

[Tenth-crew]

Thank you for your guidance these days. Without your guidance, I think it would be difficult for me to complete this task. I am sorry for asking so many stupid questions because I don't understand the front-end structure. Under your guidance, I completed this task. Thank you very much. You are an excellent open source contributor and a good teacher. This is my latest presentation result. If it meets the requirements, I am ready to review it.

success.mp4

[wxharry]

Thanks! It is nice to work with you, I am glad you finally made it and I also learned a lot myself during this process. However, we are not there yet, there are things we need to add before reviewing it carefully:

1. Add a view for this info. I know this is just one line insertion but we still need that for future maintanance. It should be easy, you can refer to other features, 99% of them are implemented this way.

2. Set proper Styles. After we move view to a separate file, we can handle its style. Currently, you set the color to black but it looks weird on my side cuz I am using a dark theme. You can also look for how the other features do this.

4. Natural insertion. For extensions, we should make it as natural as possible like it is born with the page. Let's improve it a little bit by adding a rocket icon at the begin. You can copy the icon from the repo-header-labels feature in src/pages/ContentScripts/features/repo-header-labels/view.tsx. It should be something like this (ignore the red color, we should use proper style according to the themes):

5. Comment properly. Please comment in Enghlish and remove the comments that are duplicate the code. here is a good article on writing comments, we don't have a convention for commeting but following best practices helps to keep our code clean and clear.

6. Test throughoutly. Please make sure the feature won't break when we refresh, restore, go back, etc. Just play around with it to make sure it works as expected.

BTW, Let's change the name developer-avatar-openrank to developer-hovercard-info.

[Tenth-crew]

Thank you for your comments, I have now completed the changes you requested and fixed the bugs I could find. And I tested the performance of the function in dark mode, now I believe you can see the openrank information clearly.

[wxharry]

Nice work! I have set this PR to ready to review. Thanks a lot!

There is something wrong with the insertion. It is not showing the info after a click(?). Could look into it?

2024-07-18.22.50.39-1.mov

[Tenth-crew]

I will edit it later

[Tenth-crew]

Nice work! I have set this PR to ready to review. Thanks a lot!

There is something wrong with the insertion. It is not showing the info after a click(?). Could look into it?

2024-07-18.22.50.39-1.mov

It is not clear how the bug occurred. The information can still be displayed after I click it. I will find the cause of the bug later.

[wxharry]

It is not clear how the bug occurred. The information can still be displayed after I click it. I will find the cause of the bug later.

I was testing your work. This is amazing. Thank you so much! Besides this bug, there are two chinese comments in the view index.tsx file, I think we can remove them. If you want me to help with the bug, just let me know.

[Tenth-crew]

Sorry I missed them due to my negligence, they have been removed now. Regarding the bug situation, it would be very helpful if you can find the possible cause.

[wxharry]

Sorry I missed them due to my negligence, they have been removed now. Regarding the bug situation, it would be very helpful if you can find the possible cause.

I think it is the isProcessing. What are we trying to prevent by using this isProcessing variable? Adding it at the following code fix the issue.

      if (existingDeveloperName === developerName) {
        isProcessing = false;
        return;
      }

but I got an another one while trying hovering really quick. It is not working properly somehow. We need to handle this better.

2024-07-21.01.31.33.mov

[Tenth-crew]

Sorry I missed them due to my negligence, they have been removed now. Regarding the bug situation, it would be very helpful if you can find the possible cause.

I think it is the isProcessing. What are we trying to prevent by using this isProcessing variable? Adding it at the following code fix the issue.

      if (existingDeveloperName === developerName) {
        isProcessing = false;
        return;
      }

but I got an another one while trying hovering really quick. It is not working properly somehow. We need to handle this better.

2024-07-21.01.31.33.mov

This variable was originally used to prevent re-addition. It has worked well for me before, but now there seems to be some problems. I will deal with it later.

[Tenth-crew]

I tried to move the mouse back and forth on the edge of the avatar, but the information was not repeated. However, I have added the solution you mentioned into my code.

I have used isProcessing and the developer name to prevent duplicate additions, but there is still a situation that causes duplicate additions. That is, when the mouse quickly passes over the id and avatar, the information will be added twice because it is the same name. However, in theory, the following code should prevent duplicate additions.

if (existingDeveloperName === developerName) {
        isProcessing = false;
        return;
      }
      openRankDiv?.remove();

Sometimes the information is not displayed, but I am sure that OpenRank can be found. I also found that it was successfully displayed at the beginning, but it did not display after repeated several times. I still don’t know the reason.

[wxharry]

Sometimes the information is not displayed, but I am sure that OpenRank can be found. I also found that it was successfully displayed at the beginning, but it did not display after repeated several times. I still don’t know the reason.

It is not showing because listener event functions are fired at the same time, and the latter one is prevented by the former one while the hovercard that the former one added is closed. Isprocessing prevents duplicates additions by keeping the first one and blocking the latter ones could be problematic. I add another way to cancel previous adding and keep the latest one. The idea is very simliar but I think this one is better for this case. Let me know if this works. Here is the code:

const init = async (): Promise<void> => {
  let abortController = new AbortController()
    const hovercardSelector = '[data-hovercard-type="user"]';
  document.querySelectorAll(hovercardSelector).forEach((element) => {
    // isProcessing is used to Prevent OpenRank from adding duplicates
    element.addEventListener('mouseover', async () => {
      abortController.abort();
      abortController = new AbortController();
      const signal = abortController.signal;

      const developerName = getDeveloperName(element as HTMLElement) as string;

      // Create a unique identifier for the popover
      const popoverId = `popover-${developerName}`;

      // Get the floating card container
      const $popoverContainer =
        'body > div.logged-in.env-production.page-responsive > div.Popover.js-hovercard-content.position-absolute > div > div > div';
      const popover = await elementReady($popoverContainer, { stopOnDomReady: false });

      const openRankDiv = popover?.querySelector('.openrank-info-container');
      const existingDeveloperName = openRankDiv?.getAttribute('data-developer-name');
      if (existingDeveloperName === developerName) {
        return;
      }
      openRankDiv?.remove();

      // Set the popover's unique identifier
      // make the current OpenRank information and person match
      popover?.setAttribute('data-popover-id', popoverId);

      const openrank = await getDeveloperLatestOpenrank(developerName);

      if (!openrank) {
        return;
      }

      if (!signal.aborted && popover && popover.getAttribute('data-popover-id') === popoverId) {
        // Check if the popover is still associated with the correct developer
        renderTo(popover, developerName, openrank);
      }
    });
  });
};

While this may fix the isProcessing blcoking the adding, existingDeveloperName === developerName also blocks the adding but I cannot consistently reproduce the issue (each time I refresh, it can be problematic or working perfectly until next refreshing.)

[Tenth-crew]

I have made some changes based on your code. Thanks to your code, I have a better understanding of asynchronous operations. Now the information can be displayed normally in most cases. At least I have not encountered the situation where it is no longer displayed.

[wxharry]

LGTM. Nicely done, it is a great contribution. Thank you so much!