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

change link to /terms #32

Merged
merged 2 commits into from
Sep 28, 2015
Merged

change link to /terms #32

merged 2 commits into from
Sep 28, 2015

Conversation

aislingbrock
Copy link
Contributor

What does this do?

Turns that horrible returns link that always leads to nowhere from whatever it was to /terms. Just make your terms page slug /terms and VOILA! No more pesky QA point every time anyone makes a website.

How should this be manually tested?

Related PRs / Issues / Resources?

Anything else to add? (Screenshots, background context, etc)

@kuiche
Copy link
Contributor

kuiche commented Sep 22, 2015

Not sure if hardcoding this link is the best way of "fixing" this issue. If we do it this way, we should deprecate the functionality in the returns module too.

@thomasjthomasj thoughts? Should users be using the terms and conditions route provided by the module? Or should we assume that users will use '/terms' (in which case should mothership have a route)? Should it be smarter?

I personally feel like the terms and conditions page should be in the CMS (so not using the route provided by returns) but that it should load in the page in a better way and find the slug based on the page, rather than assuming the slug will be '/terms'.

@aislingbrock
Copy link
Contributor Author

yeah, IDK, I think the current way was meant to be "smart" and obviously it was a steaming pile of crap :D

@thomasjthomasj
Copy link
Contributor

Hmm... it would be nice to have a smarter way of dealing with it but I can't see any way to do that at the moment. I think this fix is fine for now, it's the skeleton theme so people are expected to change it where they feel appropriate. That said, I think 'terms-and-conditions' makes more sense than just 'terms'

@aislingbrock
Copy link
Contributor Author

I hath changed it to terms-and-conditions

thomasjthomasj pushed a commit that referenced this pull request Sep 28, 2015
@thomasjthomasj thomasjthomasj merged commit afd4a4c into develop Sep 28, 2015
@thomasjthomasj thomasjthomasj deleted the returns-tc-link branch September 28, 2015 10:17
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.

None yet

3 participants