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

Fix: Long-string overflow [Fixes #14013] #14037

Closed

Conversation

jorgezerpa
Copy link

Description

I Created a new class ".long_ethereum_address" in global.css file. Such class has the property word-break set to break-all to allow the text to break to the next line if it is necessary.

Then I wrapped the address into an span element that has the mentionend class.

Finally I checked the same page for other languages, and found that has the same error. So I repeated the same process on all related files (for each different translation).

Related Issue -> [#14013]

…reens to avoid overflow on the layout and horizontal scroll
@github-actions github-actions bot added content 🖋️ This involves copy additions or edits translation 🌍 This is related to our Translation Program labels Oct 2, 2024
Copy link

netlify bot commented Oct 2, 2024

Deploy Preview for ethereumorg ready!

Name Link
🔨 Latest commit 2b480aa
🔍 Latest deploy log https://app.netlify.com/sites/ethereumorg/deploys/66fe88abcee7220008aa6044
😎 Deploy Preview https://deploy-preview-14037--ethereumorg.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse
7 paths audited
Performance: 48 (🟢 up 2 from production)
Accessibility: 93 (no change from production)
Best Practices: 89 (🔴 down 9 from production)
SEO: 92 (no change from production)
PWA: -
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Member

@wackerow wackerow left a comment

Choose a reason for hiding this comment

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

Hey @jorgezerpa, thanks for jumping on this.

I think this is good, just two things:

  1. @lukassim Just noting this is updating some syntax (not content) across these pages if we can pull them to Crowdin when ready
  2. @jorgezerpa We have TailwindCSS in ths repo, so let's ditch the custom class and just use the break-all class instead (see docs)

Thanks again!

ie.

<span class="break-all">`...long-string...`</span>

@wackerow wackerow changed the title [FIX issue #14013] Break string on multiple lines on small screens to avoid overflow on the layout and horizontal scroll Fix: Long-string overflow [Fixes #14013] Oct 3, 2024
Copy link
Member

@wackerow wackerow left a comment

Choose a reason for hiding this comment

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

cc: @pettinarip I think using a span here makes sense, but curious if you have any thoughts on this approach. I considered that we could apply this to the pre tag that the back-ticks will parse into, but this could result in un-intended bugs for other text that uses this syntax.

@wackerow wackerow added Update Crowdin PR introduces changes that need to be updated in Crowdin hacktoberfest Track hacktoberfest activity and removed content 🖋️ This involves copy additions or edits translation 🌍 This is related to our Translation Program labels Oct 3, 2024
@pettinarip
Copy link
Member

cc: @pettinarip I think using a span here makes sense, but curious if you have any thoughts on this approach. I considered that we could apply this to the pre tag that the back-ticks will parse into, but this could result in un-intended bugs for other text that uses this syntax.

The backtick will use a code tag, not the pre tag. So, I'd prefer to style code to accept break-word. Styling pre should not be necessary since we are already doing that within the Codeblock component.

In short, I would add a new component for code in our MdComponents, adding this style. What do you think?

…ong-ethereum-address' with tailwind class 'break-words'
@github-actions github-actions bot added content 🖋️ This involves copy additions or edits translation 🌍 This is related to our Translation Program labels Oct 3, 2024
@jorgezerpa
Copy link
Author

jorgezerpa commented Oct 3, 2024

Hello @wackerow I'm happy to hear this works!

I replaced the custom class with the tailwind class "break-words". I tried "break-all" but seems to not have effect here🤔 so I tried with "break-words" which has a similar behavior and it works well!

Hi @pettinarip I tried to edit code tag as first, but the property break-all didn't make effect, so I decided to wrap it into an span. But this approach of create a new component could works too🤔

@wackerow
Copy link
Member

wackerow commented Oct 3, 2024

@pettinarip Sorry, code not pre, but still... Personally don't think applying that on code or a custom Code is wise, since we rarely want to just chop a code snippet in the middle of a word (let me know if you disagree)... imo these long hashes are unique cases where we don't mind just letting it wrap anywhere.

I like the idea of a simple custom component, but I'd suggest not naming it "Code" and instead perhaps name it functionally as "BreakAll".

@jorgezerpa
Copy link
Author

@wackerow @pettinarip I can try create a component "BreakAll" as you mention, please confirm if it's needed 🙌

Copy link
Contributor

@nloureiro nloureiro left a comment

Choose a reason for hiding this comment

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

From the design side looks good! Thank you!!! @jorgezerpa

@corwintines
Copy link
Member

#14050

I think this resolved this more globally, so going to close this out.

@corwintines corwintines closed this Oct 4, 2024
@github-actions github-actions bot added the abandoned This has been abandoned or will not be implemented label Oct 4, 2024
@lukassim lukassim removed the Update Crowdin PR introduces changes that need to be updated in Crowdin label Oct 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
abandoned This has been abandoned or will not be implemented content 🖋️ This involves copy additions or edits hacktoberfest Track hacktoberfest activity translation 🌍 This is related to our Translation Program
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants