Conclude form design change AB test #1419
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
gbirke
approved these changes
Jan 3, 2019
| @@ -20,13 +20,26 @@ | |||
| </div> | |||
| <nav class="menu menu-main"> | |||
| <ul class="list-unstyled list-inline"> | |||
| {% for menu_item in main_menu %} | |||
There was a problem hiding this comment.
I rather liked the menu array constructed in PHP and rendered in the template as a loop. IMHO it made the template much cleaner added uniformity to the markup.
You could make the argument that the menu is skin-specific and does not belong in the PHP code. In that case I'd be fine with just reverting the template, as having "skin-specific template data" would be overkill.
If you revert the template, at least use the path function for the URL (with the route name) instead of hard-coding them.
There was a problem hiding this comment.
As discussed, since we don't really have a good place to place the main_menu items right now, we're leaving the template as is (with the addition of the path function).
timEulitz
force-pushed
the
form_redesign_test_conclusion
branch
from
e829d7c
to
207e0a6
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
https://phabricator.wikimedia.org/T212859