apertium / apertium-html-tools

Web application providing a fully localised interface for text/website/document translation, analysis and generation powered by Apertium.
http://wiki.apertium.org/wiki/Apertium-html-tools
GNU General Public License v3.0
39 stars 90 forks source link

Issue #488 #494

Closed Cellzawy closed 6 months ago

Cellzawy commented 6 months ago

Attempted to fix #488

Edited padding for form-control class in bootstrap.css to prevent the copy and clear buttons from being on top of the translated text.

Little note: I am a little new to open source so excuse me if I did something wrong.

Cellzawy commented 6 months ago

Please don't edit bootstrap.css. It's generated from upstream sources. There a number of other CSS files that can be edited instead with specific styles. Or, use inline utility classes.

Noted, sorry for that. Working on it now

Cellzawy commented 6 months ago

I have removed my edits on bootstrap.css like you asked, added pt-4 class to both of the Form.Controls in TextTranslationForm.tsx This should prevent the buttons from being on top of the text, let me know if you'd like more spacing though.

ahmedsiam0 commented 6 months ago

I think more spacing is needed. When hovering, the button overlaps with the text. hover

Cellzawy commented 6 months ago

I think more spacing is needed. When hovering, the button overlaps with the text. hover

Fair enough, to increase the spacing I would give it pr-5 instead of pr-4. This is how it would look: image

Or, if pr-5 it's too much space I can create my own class (let's name it pr-4-5 for example) and give it a padding that's in between pr-4 and pr-5. It would look like this: image

Let me know which one you prefer.

ahmedsiam0 commented 6 months ago

The second option looks visually better for me. (between pr-4 and pr-5).

coveralls commented 6 months ago

Pull Request Test Coverage Report for Build 8353441372

Details


Totals Coverage Status
Change from base Build 8313816702: 0.0%
Covered Lines: 1381
Relevant Lines: 1396

💛 - Coveralls
Cellzawy commented 6 months ago

The second option looks visually better for me. (between pr-4 and pr-5).

Done, waiting for your feedback.

Cellzawy commented 6 months ago

I'm now seeing a large unnecessary padding on the translated text:

image

Right, apparently it's because of the difference in the interface language's direction, sorry I have missed that part. Fixed it by setting padding on left if it's an RTL language and the opposite for LTR.

Waiting for your feedback.