-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Conversation
jacogr
commented
Dec 8, 2016
•
edited
Loading
edited
- Use new ~ URLs where applicable/missed
- Removed unused files
- Move files to correct locations
- Combine similar functionality into one file, multiple exports
- Remove unused styles
@@ -15,17 +15,6 @@ | |||
// along with Parity. If not, see <http://www.gnu.org/licenses/>. | |||
|
|||
import { stringify } from 'querystring'; | |||
import React from 'react'; | |||
|
|||
export const termsOfService = ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I put this here on purpose. The terms of service describe the legal implications of using the 3rd party service in 3rdparty/sms-verification
. I can understand that having React in here feels weird, but since termsOfService
is stateless & static, I think it's worth it. Having it in modals/SMSVerification/GatherData
is very unintuitive imho.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very weird indeed, since the 3rdparty libraries are also stuff that we would like to start publishing under the @parity/
namespace. Unless we make an exception for the specific one and don't publish it at all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, will revert that one, but breaks the purpose or the area. (Will have to treat it as "special and internal only")
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess we have three choices.
- not bundling the legal text with the library
- using sth like
dangerouslySetInnerHTML
. - using plain text legal terms. they look less nice and the links are gone, but we still get rid of React.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suppose on #3, we could go with something like markdown - will actually kill 2 birds with one stone -
- be readable as text
- be transformable into pretty HTML
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Markdown should get compiled on build. What about the template system currently in place? One you combine it with dangerouslySetInnerHTML
.
Disagree with moving of |
This reverts commit 3379e61.