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

Replace $app->url() with url-generated link to index #2302

Merged
merged 2 commits into from
Sep 24, 2020

Conversation

askvortsov1
Copy link
Sponsor Member

The url method of Container was removed in #2142. Works locally.

Copy link
Member

@clarkwinkelmann clarkwinkelmann left a comment

Choose a reason for hiding this comment

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

I think everywhere else we make a link to the homepage we use ->to('forum')->base() https://github.com/flarum/core/blob/df8f73bd3d656e073dcfff5bddd8e7692ca19162/src/Api/Serializer/ForumSerializer.php#L67

Or an alternative could be to use the default named route https://github.com/flarum/core/blob/2431df5602e218d77388cec0019d13a473912be0/src/Forum/ForumServiceProvider.php#L207

Using index would create a link to /all when a different homepage has been selected I believe.

@askvortsov1
Copy link
Sponsor Member Author

Good catch, thanks!

@askvortsov1 askvortsov1 merged commit cc48e9a into master Sep 24, 2020
@askvortsov1 askvortsov1 deleted the as/fix-app-url-method branch September 24, 2020 15:30
@franzliedke franzliedke removed this from the 0.1.0-beta.14 milestone Sep 24, 2020
@franzliedke
Copy link
Contributor

Removed this from the milestone as this is just a regression from another change and not relevant for the changelog or anything like that, IMO. I hope you don't mind.

@askvortsov1
Copy link
Sponsor Member Author

Thanks! I only added it so that we don't forget to fix it before release. The milestone will need a comb-over before release anyways

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.

4 participants