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

Implement default welcome page and allow custom URL /w config #4015

Merged
merged 3 commits into from
May 25, 2017

Conversation

lukebarnard1
Copy link
Contributor

Copy link
Member

@dbkr dbkr left a comment

Choose a reason for hiding this comment

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

lgtm apart from comments

@@ -11,5 +11,6 @@
"matrix.org"
]
},
"welcomeUserId": "@RiotBot:matrix.org"
"welcomeUserId": "@RiotBot:matrix.org",
"welcomePageUrl": "https://about.riot.im"
Copy link
Member

Choose a reason for hiding this comment

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

doesn't this want to be home.html?

Copy link
Member

Choose a reason for hiding this comment

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

Oh I see, home.html is the default if one isn't defined. I think this is going to provide quite strange behaviour for people who just copy the sample config assuming the values in it are sane (ie. a lot of people probably)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fair enough, I'll just remove it

teamServerUrl: React.PropTypes.string.isRequired,
teamToken: React.PropTypes.string.isRequired,
collapsedRhs: React.PropTypes.bool,
// Team token. Optional. If unset, the homePageUrl will be used
Copy link
Member

Choose a reason for hiding this comment

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

url will be used as the team token?

Copy link
Member

Choose a reason for hiding this comment

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

Oh, I see what's going on. Comment could probably make this clearer. :)

<body>
<div>Welcome to Riot</div>
</body>
</html>
Copy link
Member

Choose a reason for hiding this comment

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

What's the long-term intention here? Presumably not a minimal, css-less page?

Copy link
Contributor Author

@lukebarnard1 lukebarnard1 May 25, 2017

Choose a reason for hiding this comment

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

Tom and Ben are working on it (https://lant.uk/misc/welcome/) and we'll have something before new-guest-access goes live

@dbkr dbkr assigned lukebarnard1 and unassigned dbkr May 24, 2017
@lukebarnard1 lukebarnard1 merged commit 36ec156 into new-guest-access May 25, 2017
@t3chguy t3chguy deleted the luke/new-guest-access-default-welcome branch May 12, 2022 08:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants