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

Code diff UI goes misalignment with code review comments #22911

Closed
wxiaoguang opened this issue Feb 15, 2023 · 9 comments · Fixed by #23096
Closed

Code diff UI goes misalignment with code review comments #22911

wxiaoguang opened this issue Feb 15, 2023 · 9 comments · Fixed by #23096
Labels
topic/ui Change the appearance of the Gitea UI type/bug
Milestone

Comments

@wxiaoguang
Copy link
Contributor

wxiaoguang commented Feb 15, 2023

Description

https://jsfiddle.net/ob0y2x83/

The padding inside td makes the widths of td become larger.

<table style="width: 100%">
  <tr>
    <td style="width: 10px;">a</td>
    <td style="width: 0"></td>
    <td style="width: 10px;">b</td>
    <td style="width: 49%">long content</td>
    
    <td style="width: 10px;">a</td>
    <td style="width: 0"></td>
    <td style="width: 10px;">b</td>
    <td style="width: 49%">long content</td>
  </tr>
  <tr>
    <td colspan="4">
      <div style="width: 30%"> 
        <div style="padding-left: 100px;">
          with padding
        </div>
      </div>
    </td>
    <td colspan="4">
      <div style="width: 30%"> 
        <div style="padding-left: 100px;">
          with padding
        </div>
      </div>
    </td>
  </tr>
</table>

<hr>

<table style="width: 100%">
  <tr>
    <td style="width: 10px;">a</td>
    <td style="width: 0"></td>
    <td style="width: 10px;">b</td>
    <td style="width: 49%">long content</td>
    
    <td style="width: 10px;">a</td>
    <td style="width: 0"></td>
    <td style="width: 10px;">b</td>
    <td style="width: 49%">long content</td>
  </tr>
  <tr>
    <td colspan="4">
      <div style="width: 30%"> 
        <div style="padding-left: 0;">
          no padding
        </div>
      </div>
    </td>
    <td colspan="4">
      <div style="width: 30%"> 
        <div style="padding-left: 0;">
          no padding
        </div>
      </div>
    </td>
  </tr>
</table>

image

image

@jpraet jpraet added the topic/ui Change the appearance of the Gitea UI label Mar 5, 2023
@lunny lunny added this to the 1.19.1 milestone Mar 20, 2023
@wxiaoguang
Copy link
Contributor Author

wxiaoguang commented Mar 30, 2023

Move the milestone to 1.20 or 1.x ? I am not working on it. Welcome to pick it.

Update: I proposed a draft #23096 , welcome to test & review.

@lunny
Copy link
Member

lunny commented Mar 30, 2023

Move the milestone to 1.20 or 1.x ? I am not working on it. Welcome to pick it.

When we are releasing 1.19.1 if there is no PR to fix it. We can move it to 1.19.2

@wxiaoguang
Copy link
Contributor Author

I think there needs a document to explain how the milestone works.

In my mind, only necessary/blocker issues should be on the stable release milestones.

@lunny
Copy link
Member

lunny commented Mar 30, 2023

I think there needs a document to explain how the milestone works.

In my mind, only necessary/blocker issues should be on the stable release milestones.

Yes, we need one. I think since it has been marked bug so that it could be put in a minor version if 1.19 has been affected.

@wxiaoguang
Copy link
Contributor Author

wxiaoguang commented Mar 30, 2023

This problem is very complicated. I don't think it could be backported easily even if it gets resolved in 1.20

@lunny lunny modified the milestones: 1.19.1, 1.20.0 Mar 31, 2023
@lunny
Copy link
Member

lunny commented Mar 31, 2023

OK. So let's move it to 1.20

@jpraet
Copy link
Member

jpraet commented Mar 31, 2023

I do think fixing this UX regression deserves some attention though. This incorrect alignment in the side-by-side diff for review comments is quite annoying. Especially for the + button to add a new review comment. Personally, I switched to the unified diff view on my company instance just to work around this issue.

Does this seem like something in your wheelhouse maybe @silverwind, as you've been tackling some other UI issues related to the PR diff view lately?

@silverwind
Copy link
Member

Possible, but personally I do not use side-by-side ever, so it's not a priority for me.

@wxiaoguang
Copy link
Contributor Author

wxiaoguang commented Apr 1, 2023

@jpraet I made more improvements in #23096 , I guess it's good enough now (I haven't fully tested it with various browsers), and I think it's safe for you to test it in your production environment.

If it looks good to you, please help to review. Thank you.

@wxiaoguang wxiaoguang changed the title Show the problem of the misalignment of code review comments Code diff UI goes misalignment with code review comments Apr 1, 2023
lunny pushed a commit that referenced this issue Apr 4, 2023
GiteaBot pushed a commit to GiteaBot/gitea that referenced this issue Apr 4, 2023
6543 pushed a commit that referenced this issue Apr 4, 2023
Backport #23096 by @wxiaoguang

Close #22911

I think it's ready for review now, feel free to test it, welcome to help
to improve.

### Before


![image](https://user-images.githubusercontent.com/2114189/220958734-06871615-b498-4143-8449-3d443f08ffaa.png)

### After


![image](https://user-images.githubusercontent.com/2114189/220958621-0dce2728-57b8-4a1f-ac5d-48c7c2d42f5c.png)

Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
topic/ui Change the appearance of the Gitea UI type/bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants