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

Bug 705416 - Port Firefox Release Notes index page to Bedrock and automate the update #968

Merged
merged 1 commit into from Aug 2, 2013
Merged

Bug 705416 - Port Firefox Release Notes index page to Bedrock and automate the update #968

merged 1 commit into from Aug 2, 2013

Conversation

kyoshino
Copy link
Contributor

No description provided.

@openjck
Copy link
Contributor

openjck commented Jun 20, 2013

Will need a more detailed review.

@sgarrity
Copy link
Contributor

A few notes based on group review:

  1. The main headline/paragraph are not aligned with the grid (needs 10px left margin - see other similar pages)
  2. These unordered-lists should probably be ordered lists.
  3. Layout suggestion: Float the major versions in a column and float al minor versions in a separate column, so the minor versions never wrap under the major versions.

@kyoshino
Copy link
Contributor Author

Thanks for the feedback. Will fix them later.

@kyoshino
Copy link
Contributor Author

Fixed the layout and markup.

@sgarrity
Copy link
Contributor

Fixed the layout and markup.

Looks much better. Could the width of the columns be smaller? Maybe 6em rather than 8em? In the mobile layouts, the current widths are a bit large.

@kyoshino
Copy link
Contributor Author

Adjusted the width of the columns as per @sgarrity's comment.

{% block breadcrumbs %}
<nav class="breadcrumbs">
<a href="{{ url('mozorg.home') }}">{{ _('Home') }}</a> >
<a href="/firefox/">{{ _('Firefox') }}</a> >
Copy link
Contributor

Choose a reason for hiding this comment

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

Really small tweak - this href should probably be {{ url('firefox') }}. Should be good to merge after that.

@kyoshino
Copy link
Contributor Author

kyoshino commented Aug 2, 2013

@jpetto Thanks! Replaced /firefox/ with {{ url('firefox') }}.

jpetto added a commit that referenced this pull request Aug 2, 2013
Bug 705416 - Port Firefox Release Notes index page to Bedrock and automate the update
@jpetto jpetto merged commit 3a37119 into mozilla:master Aug 2, 2013
@kyoshino kyoshino deleted the bug-705416-relnotes-index branch August 2, 2013 18:39
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

4 participants