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.

Sign up for GitHub

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

Jump to bottom

Lastpost (MessageIndex) alignment issue / Breaking things #6089

Merged
merged 3 commits into from Nov 7, 2020
Merged

Lastpost (MessageIndex) alignment issue / Breaking things #6089

merged 3 commits into from Nov 7, 2020

Conversation

XinYenFon
Copy link
Contributor

@XinYenFon XinYenFon commented Apr 27, 2020 •
edited

fixes #6044
fixes #6071

@XinYenFon XinYenFon changed the title fixes #6044 Lastpost (MessageIndex) alignment issue Apr 27, 2020
@XinYenFon XinYenFon changed the title Lastpost (MessageIndex) alignment issue Lastpost (MessageIndex) alignment issue / Breaking things Apr 27, 2020
@sbulen sbulen added the Theme label Apr 27, 2020
live627 referenced this pull request May 13, 2020
@Sesquipedalian
Improves display of long strings in error log messages
a7ade21 
Signed-off-by: Jon Stovell <jonstovell@gmail.com>
Sesquipedalian
Sesquipedalian previously requested changes May 13, 2020
View reviewed changes
Copy link
Member

@Sesquipedalian Sesquipedalian left a comment

Choose a reason for hiding this comment

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

Let's also change the CSS for the word_break class from word-break: break-all to word-wrap: break-word. It achieves the same goal, but it's less disruptive in the process.

@XinYenFon
Copy link
Contributor Author

not sure about that... afaik there are some cases break-word fails due to some weird combination of \/ that's why we agreed on breaking all... (also why break partially when you can break whole glass in one push...)

@Sesquipedalian
Copy link
Member

not sure about that... afaik there are some cases break-word fails due to some weird combination of \/ that's why we agreed on breaking all...

Fair enough, but then I don't think it is a good idea to apply word-break: break-all to the entire error_info div. We only want that applied to specific strings within the div—just the URLs, really. The error message would be better handled by word-wrap: break-word plus overflow:auto to handle any rare case where a browser bug messes up the word breaking for whatever reason.

@Sesquipedalian Sesquipedalian mentioned this pull request May 13, 2020
Merged
@XinYenFon
Copy link
Contributor Author

not sure about that... afaik there are some cases break-word fails due to some weird combination of \/ that's why we agreed on breaking all...

Fair enough, but then I don't think it is a good idea to apply word-break: break-all to the entire error_info div. We only want that applied to specific strings within the div—just the URLs, really. The error message would be better handled by word-wrap: break-word plus overflow:auto to handle any rare case where a browser bug messes up the word breaking for whatever reason.

we can handle the situation far better in next version, rework the area to show better info with better output (maybe even with -copy- button?) but for now its -imo- best to merge this as is.

@MissAllSunday MissAllSunday modified the milestones: RC3, Final Jul 20, 2020
@XinYenFon @MissAllSunday
fixes SimpleMachines#6044
ee500a5
@XinYenFon @MissAllSunday
If you have a chance to break something why do it one by one... do it…
15e1e7c 
… as whole... just like the glass they put next to table... You will be forgotten glass!

fixes SimpleMachines#6071
fix: indent tabs
1489bc9 
Signed-off-by: Jessica González <suki@missallsunday.com>
@MissAllSunday MissAllSunday merged commit 70f5dcb into SimpleMachines:release-2.1 Nov 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MissAllSunday MissAllSunday MissAllSunday approved these changes

@SychO9 SychO9 SychO9 approved these changes

@Sesquipedalian Sesquipedalian Sesquipedalian left review comments

Assignees
No one assigned
Labels
Theme
Projects
None yet
Milestone
RC4
Development

Successfully merging this pull request may close these issues.

Error log display Minor alignment issue - should get date & time on one line
5 participants
@XinYenFon @Sesquipedalian @MissAllSunday @SychO9 @sbulen