Skip to content

Add support for URL locale filter to target specific locale#676

Merged
aduth merged 1 commit intomainfrom
aduth-locale-url-filter-target
Jul 13, 2021
Merged

Add support for URL locale filter to target specific locale#676
aduth merged 1 commit intomainfrom
aduth-locale-url-filter-target

Conversation

@aduth
Copy link
Copy Markdown
Contributor

@aduth aduth commented Jul 9, 2021

Context: #674 (comment)

Why: To simplify language picker URL logic.

@zachmargolis
Copy link
Copy Markdown
Contributor

Spec failure seems like we ate the wrong part of a URL

- /home/circleci/project/_site/create-an-account/index.html
  *  External link http://help/authentication-methods/which-authentication-method-should-i-use/ failed: response code 0 means something's wrong.

@zachmargolis
Copy link
Copy Markdown
Contributor

zachmargolis commented Jul 9, 2021

Spec failure seems like we ate the wrong part of a URL

I took a look at this and found an somewhere an extra // was left in... so I made a really hacky patch 5ced333, feel free to rebase/squash it out

update: also needed 5545ed6 🙈

@aduth
Copy link
Copy Markdown
Contributor Author

aduth commented Jul 12, 2021

Spec failure seems like we ate the wrong part of a URL

I took a look at this and found an somewhere an extra // was left in... so I made a really hacky patch 5ced333, feel free to rebase/squash it out

update: also needed 5545ed6 🙈

Nice, thanks for giving it a look. I'd put together a spec file toward the end of the day on Friday and it looks like your changes work well 👍 Pushed a few other changes as well.

Copy link
Copy Markdown
Contributor

@zachmargolis zachmargolis left a comment

Choose a reason for hiding this comment

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

LGTM

Comment on lines +1 to +4
require 'jekyll'
require './_plugins/locale_url_filter.rb'

class JekyllFilter
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

😻 specs for plugins!

@aduth aduth marked this pull request as ready for review July 12, 2021 16:05
Copy link
Copy Markdown
Contributor

@zachmargolis zachmargolis left a comment

Choose a reason for hiding this comment

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

LGTM

**Why**: To simplify language picker URL logic

Co-Authored-By: Zach Margolis <zbmargolis@gmail.com>
@aduth aduth force-pushed the aduth-locale-url-filter-target branch from 052d26d to b7d1c0b Compare July 12, 2021 17:47
@aduth aduth merged commit a5a966e into main Jul 13, 2021
@aduth aduth deleted the aduth-locale-url-filter-target branch July 13, 2021 14:21
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