Skip to content

Fix comment box not showing on PR commit replies #5239

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

Closed
wants to merge 3 commits into from
Closed

Fix comment box not showing on PR commit replies #5239

wants to merge 3 commits into from

Conversation

jamesa
Copy link
Contributor

@jamesa jamesa commented Oct 31, 2018

Fixes #5237

The form that gets revealed when clicking on the "Reply" button mentioned in #5237, and the button, do not share the same parent. The button ends up getting wrapped in a link which causes this issue.

I have no idea how or why this worked on the English translation though. Didn't dig in that deep. :)

@codecov-io
Copy link

codecov-io commented Oct 31, 2018

Codecov Report

❗ No coverage uploaded for pull request base (master@e8b197d). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #5239   +/-   ##
=========================================
  Coverage          ?   37.39%           
=========================================
  Files             ?      312           
  Lines             ?    46313           
  Branches          ?        0           
=========================================
  Hits              ?    17317           
  Misses            ?    26517           
  Partials          ?     2479

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e8b197d...137116b. Read the comment docs.

@bkcsoft bkcsoft added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Oct 31, 2018
@techknowlogick techknowlogick added this to the 1.7.0 milestone Nov 1, 2018
@lunny lunny added the topic/ui Change the appearance of the Gitea UI label Nov 1, 2018
@bkcsoft bkcsoft added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Nov 2, 2018
@zeripath
Copy link
Contributor

zeripath commented Nov 6, 2018

Hmm.. it's very interesting in the English translation the comment button appears in the following structure (This is presumably the case for every translation that works):

<div id="code-comments-...">
    <div class="ui comments">...</div>
    <button class="comment-form-reply ui green labeled icon tiny button">...</button>
    <form class="ui form hide comment-form comment-form-reply" action="..." method="post">...</form>
</div>

However, in the Spanish translation it becomes:

<div id="code-comments-X">
    <div class="ui comments">...</div>
    <a href="#issuecomment-X">
        <button class="comment-form-reply ui green labeled icon tiny button">...</button>
    </a>
    <form class="ui form hide comment-form comment-form-reply" action="..." method="post">...</form>
</div>

@zeripath
Copy link
Contributor

zeripath commented Nov 6, 2018

OK I think that the locale-es_ES.ini file has a problem. There are likely other problems in that file too.

line 459:

...
issues.commented_at=`comentado <a href="#%s"> %s`</a>
...

is the cause of this issue. The backtick (`) before the </a> means that the <a href="..."> is never closed.

@techknowlogick techknowlogick added the status/blocked This PR cannot be merged yet, i.e. because it depends on another unmerged PR label Nov 6, 2018
@jamesa
Copy link
Contributor Author

jamesa commented Nov 6, 2018

Nice find! This might not be the best fix then. It works but it might not be the most efficient.

Copy link
Member

@techknowlogick techknowlogick left a comment

Choose a reason for hiding this comment

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

Seems as though the translation files may be an issue. See: #5237 (comment)

Copy link
Contributor

@zeripath zeripath left a comment

Choose a reason for hiding this comment

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

This PR is attempting to fix the problem in the wrong place. The issue lies in the locale file for Spanish and the problem should be fixed in the translation.

@jamesa
Copy link
Contributor Author

jamesa commented Nov 6, 2018

If you guys are fine with it, I can use this PR to make those changes. Unless @zeripath you would like to, or if another PR should be raised for this issue.

@techknowlogick
Copy link
Member

The translations should be done via crowdin, as otherwise they will be overwritten by crowdin.

@zeripath
Copy link
Contributor

zeripath commented Nov 6, 2018

line 795: auths.tip.facebook also needs adjustment as it has an open double-quote.

@techknowlogick
Copy link
Member

Closing as translation files have been updated.

@jamesa jamesa deleted the fix-comment-box branch November 8, 2018 12:16
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/need 1 This PR needs approval from one additional maintainer to be merged. status/blocked This PR cannot be merged yet, i.e. because it depends on another unmerged PR topic/ui Change the appearance of the Gitea UI type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Reply button disappear on click in PR when language is spanish
7 participants