Skip to content
This repository has been archived by the owner on Jul 31, 2024. It is now read-only.

Add descscription to client #2282

Closed
wants to merge 6 commits into from
Closed

Add descscription to client #2282

wants to merge 6 commits into from

Conversation

LindaLawton
Copy link
Contributor

What issue does this PR address?
2280

Does this PR introduce a breaking change?

nope

Please check if the PR fulfills these requirements

  • The commit follows our guidelines
  • Unit Tests for the changes have been added (for bug fixes / features)

Other information:

Made some changes to the consent screen to display this new info in the Host project. The rest should be pretty basic.

@@ -8,9 +8,19 @@
<div class="client-logo"><img src="@Model.ClientLogoUrl"></div>
Copy link
Contributor Author

@LindaLawton LindaLawton May 1, 2018

Choose a reason for hiding this comment

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

Note. I have urls inserted here and this isnt working due to CORS errors. Not sure what you have been inserting. URL is probably a bad idea on second thought ignore this.

Tweeking the building of the name.    Oneline if FTW
@brockallen brockallen self-assigned this May 1, 2018
@Model.ClientName
<small>is requesting your permission</small>
</h1>
<h1>@Html.Raw((string.IsNullOrWhiteSpace(Model.ClientUrl))? $"{@Model.ClientName}" : $"<a href='{@Model.ClientUrl}'>{@Model.ClientName}</a>") </h1>
Copy link
Member

Choose a reason for hiding this comment

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

I don't like Html.Raw in here -- XSS potential.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I haven't done web development in 10 years so i am a bit behind in what one should be using.

Do you have a suggestion?

Copy link
Member

@brockallen brockallen Jun 5, 2018

Choose a reason for hiding this comment

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

I'd just write the if/else blocks to build the html tags. The use of Raw disables the HTML encoding of the value.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I wont have time to do this until Thursday. Up to you if you can wait it looks like your on a clean out PR spree today.

Copy link
Member

Choose a reason for hiding this comment

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

No, I am just looking at issues. No time yet for me to do any code :)

@brockallen
Copy link
Member

Given that we reworked the models last week, I'll close this and add the description manually. Thanks.

@brockallen brockallen closed this Aug 3, 2018
@lock
Copy link

lock bot commented Jan 10, 2020

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants