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

Reintroduce word wrapping to the custom login message #12460

Merged
merged 1 commit into from
Jan 23, 2021
Merged

Reintroduce word wrapping to the custom login message #12460

merged 1 commit into from
Jan 23, 2021

Conversation

efelon
Copy link
Contributor

@efelon efelon commented Jan 23, 2021

The <pre> tag breaks the well formed output of the login messages by introducing scrollbars and wrong background color. Later is most noticeable in the dark theme.

before after
grafik grafik
grafik grafik

Just by using div instead of pre fixes the output.

DO NOT DELETE THE UNDERLYING TEXT

Please note

Please read this information carefully. You can run ./lnms dev:check to check your code before submitting.

  • Have you followed our code guidelines?
  • If my Pull Request does some changes/fixes/enhancements in the WebUI, I have inserted a screenshot of it.

Testers

If you would like to test this pull request then please run: ./scripts/github-apply <pr_id>, i.e ./scripts/github-apply 5926
After you are done testing, you can remove the changes with ./scripts/github-remove. If there are schema changes, you can ask on discord how to revert.

The <pre> tag breaks the well formed output of the login messages by introducing scrollbars and wrong background color. Later is most noticeable in the dark theme.
@SourceDoctor SourceDoctor self-requested a review January 23, 2021 23:52
Copy link
Member

@SourceDoctor SourceDoctor left a comment

Choose a reason for hiding this comment

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

LGTM

@SourceDoctor SourceDoctor merged commit 3455371 into librenms:master Jan 23, 2021
@murrant
Copy link
Member

murrant commented Jan 26, 2021

@efelon this breaks line returns in the message

@efelon
Copy link
Contributor Author

efelon commented Jan 26, 2021

@murrant Can you give me more details how you entered the new lines?

What I have done is to check the "General Authentication Settings" input field for "Logon Message". There you don't have the possibility to enter new lines. After your comment I tried also:
image
Which will print "\n" and <br> just as characters. I also tried: &#13;&#10;

@murrant
Copy link
Member

murrant commented Jan 27, 2021

@efelon Like this:

line one
line two

That is why it was changed to a pre tag a couple of releases ago ;)

If you want both, you will need to have use nl2br and html purifier to allow only br (and maybe a few select other) tags.

@efelon
Copy link
Contributor Author

efelon commented Jan 27, 2021

@murrant I see, my bad. I haven't checked the option in the configuration config.php:

$config['login_message']    = "Unauthorised access or use 
shall render the user liable to 
criminal and/or civil prosecution.";

Let me see if I can find a solution otherwise I will make a new PR reverting the change.

@efelon
Copy link
Contributor Author

efelon commented Jan 27, 2021

I found a solution using <div> and white-space: pre-wrap; working on firefox and chrome. I create another PR later.

image

efelon added a commit to efelon/librenms that referenced this pull request Jan 27, 2021
Introducing own css class for customization

Fix problem introduced with librenms#12460
murrant pushed a commit that referenced this pull request Jan 27, 2021
Introducing own css class for customization

Fix problem introduced with #12460
@murrant murrant added the WebUI label Feb 2, 2021
@librenms-bot
Copy link

This pull request has been mentioned on LibreNMS Community. There might be relevant details there:

https://community.librenms.org/t/21-1-0-changelog/14698/1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants