-
Notifications
You must be signed in to change notification settings - Fork 128
Fixes #4848 - Header Without text labels #4940
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,7 +21,7 @@ exports.Header = function Header(props) { | |
|
||
exports.Header.propTypes = { | ||
hasLogo: PropTypes.bool, | ||
children: PropTypes.node.isRequired, | ||
children: PropTypes.node, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This addresses server warning in logs when Header doesn't have children |
||
isOwner: PropTypes.bool, | ||
hasFxa: PropTypes.bool, | ||
shouldGetFirefox: PropTypes.bool, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,20 +17,20 @@ exports.HomePageHeader = class HomePageHeader extends React.Component { | |
renderFxASignIn() { | ||
return ( | ||
this.props.isFirefox && this.props.isOwner ? | ||
<SignInButton isAuthenticated={this.props.hasFxa} initialPage="" /> : null | ||
<SignInButton isAuthenticated={this.props.hasFxa} initialPage="" | ||
staticLink={this.props.staticLink} /> : null | ||
); | ||
} | ||
|
||
render() { | ||
let myShots; | ||
if (this.props.isOwner) { | ||
myShots = <Localized id="shotIndexPageMyShotsButton" attrs={{title: true}}> | ||
<a className="nav-button icon-shots" title="My Shots" href="/shots" onClick={ this.onClickMyShots.bind(this) } tabIndex="0"> | ||
<Localized id="gMyShots"> | ||
<span>My Shots</span> | ||
</Localized> | ||
</a> | ||
</Localized>; | ||
<a className="nav-button icon-shots" title="My Shots" href="/shots" | ||
onClick={ this.onClickMyShots.bind(this) } tabIndex="0"> | ||
<img src={this.props.staticLink("/static/img/icon-shots.svg")} /> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should these images have alt text? I'm not sure There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure either, in share-button we have similar tags where img is embedded inside link and have omitted img alt in favor of title attribute on link. https://github.com/mozilla-services/screenshots/blob/master/server/src/share-buttons.js#L129 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Cool, we can leave it for now, and see if it triggers the a11y linter later There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should add alt text to all of our images for a11y. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Filed #4976 to take it up in Q4 with a11y bugs |
||
</a> | ||
</Localized>; | ||
} | ||
|
||
const signin = this.renderFxASignIn(); | ||
|
@@ -49,4 +49,5 @@ exports.HomePageHeader.propTypes = { | |
hasFxa: PropTypes.bool, | ||
isOwner: PropTypes.bool, | ||
isFirefox: PropTypes.bool, | ||
staticLink: PropTypes.func, | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -363,68 +363,63 @@ class Body extends React.Component { | |
let favoriteShotButton = null; | ||
let trashOrFlagButton = null; | ||
let editButton = null; | ||
const highlight = this.state.highlightEditButton | ||
? <div className="edit-highlight" | ||
onClick={this.onClickEdit.bind(this)} | ||
onMouseOver={this.onMouseOverHighlight.bind(this)} | ||
onMouseOut={this.onMouseOutHighlight.bind(this)}></div> | ||
: null; | ||
const activeFavClass = this.props.expireTime ? "" : "is-fav"; | ||
const inactive = this.props.isFxaAuthenticated ? "" : "inactive"; | ||
|
||
favoriteShotButton = <div className="favorite-shot-button"> | ||
<Localized id="shotPagefavoriteButton" attrs={{title: true}}> | ||
<button className={`nav-button ${inactive}`} | ||
disabled={!this.props.isFxaAuthenticated} | ||
onClick={this.onClickFavorite.bind(this)}> | ||
<span className={`icon-favorite favorite ${activeFavClass}`} ></span> | ||
<Localized id="shotPageFavorite"> | ||
<span className={`favorite-text favorite ${activeFavClass} `}>Favorite</span> | ||
</Localized> | ||
</button></Localized></div>; | ||
|
||
const downloadButton = <div className="download-shot-button"> | ||
<Localized id="shotPageDownloadShot" attrs={{title: true}}> | ||
<button className="nav-button icon-download" onClick={this.onClickDownload.bind(this)} | ||
title="Download the shot image"> | ||
<Localized id="shotPageDownload"> | ||
<span>Download</span> | ||
</Localized> | ||
</button> | ||
</Localized></div>; | ||
|
||
const copyButton = <div className="copy-img-button" hidden={this.state.isClient && !this.state.canCopy}> | ||
<Localized id="shotPageCopyButton" attrs={{title: true}}> | ||
<button className="nav-button icon-copy transparent copy" | ||
title="Copy image to clipboard" | ||
onClick={this.onClickCopy.bind(this)}> | ||
<Localized id="shotPageCopy"> | ||
<span>Copy Image</span> | ||
</Localized> | ||
</button> | ||
</Localized> | ||
</div>; | ||
let downloadButton = null; | ||
let copyButton = null; | ||
|
||
if (this.props.isOwner) { | ||
const highlight = this.state.highlightEditButton | ||
? <div className="edit-highlight" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe we should replace this svg highlight with a separate highlighted-pen image (fine to do this in a followup bug) |
||
onClick={this.onClickEdit.bind(this)} | ||
onMouseOver={this.onMouseOverHighlight.bind(this)} | ||
onMouseOut={this.onMouseOutHighlight.bind(this)}> | ||
</div> : null; | ||
const favImgSrc = this.props.expireTime ? this.props.staticLink("/static/img/icon-heart-outline.svg") : | ||
this.props.staticLink("/static/img/icon-heart.svg"); | ||
const inactive = this.props.isFxaAuthenticated ? "" : "inactive"; | ||
|
||
favoriteShotButton = <div className="favorite-shot-button"> | ||
<Localized id="shotPagefavoriteButton" attrs={{title: true}}> | ||
<button className={`button transparent nav-button ${inactive}`} | ||
disabled={!this.props.isFxaAuthenticated} onClick={this.onClickFavorite.bind(this)}> | ||
<img src={favImgSrc} /> | ||
</button> | ||
</Localized></div>; | ||
|
||
trashOrFlagButton = <DeleteShotButton | ||
clickDeleteHandler={ this.clickDeleteHandler.bind(this) } | ||
confirmDeleteHandler={ this.confirmDeleteHandler.bind(this) } | ||
cancelDeleteHandler={ this.cancelDeleteHandler.bind(this) } />; | ||
clickDeleteHandler={this.clickDeleteHandler.bind(this)} | ||
confirmDeleteHandler={this.confirmDeleteHandler.bind(this)} | ||
cancelDeleteHandler={this.cancelDeleteHandler.bind(this)} | ||
staticLink={this.props.staticLink} />; | ||
|
||
editButton = this.props.enableAnnotations ? <div className="edit-shot-button"> | ||
<Localized id="shotPageEditButton" attrs={{title: true}}> | ||
<button className="nav-button icon-edit transparent edit" | ||
title="Edit this image" | ||
onClick={this.onClickEdit.bind(this)} | ||
ref={(edit) => { this.editButton = edit; }}> | ||
<Localized id="shotPageDraw"> | ||
<span>Draw</span> | ||
</Localized> | ||
<button className="button transparent nav-button" | ||
title="Edit this image" | ||
onClick={this.onClickEdit.bind(this)} | ||
ref={(edit) => { this.editButton = edit; }}> | ||
<img src={this.props.staticLink("/static/img/icon-pen.svg")} /> | ||
</button> | ||
</Localized> | ||
<PromoDialog promoClose={this.promoClose.bind(this)} display={this.state.promoDialog} /> | ||
{ highlight } | ||
</div> : null; | ||
|
||
downloadButton = <div className="download-shot-button"> | ||
<Localized id="shotPageDownloadShot" attrs={{title: true}}> | ||
<button className={`button transparent nav-button`} onClick={this.onClickDownload.bind(this)} | ||
title="Download the shot image"> | ||
<img src={this.props.staticLink("/static/img/icon-download.svg")} /> | ||
</button> | ||
</Localized></div>; | ||
|
||
copyButton = <div className="copy-img-button" hidden={this.state.isClient && !this.state.canCopy}> | ||
<Localized id="shotPageCopyButton" attrs={{title: true}}> | ||
<button className="button nav-button transparent copy" | ||
title="Copy image to clipboard" | ||
onClick={this.onClickCopy.bind(this)}> | ||
<img src={this.props.staticLink("/static/img/icon-copy.svg")} /> | ||
</button> | ||
</Localized></div>; | ||
} | ||
|
||
let clip; | ||
|
@@ -447,7 +442,8 @@ class Body extends React.Component { | |
<reactruntime.BodyTemplate {...this.props}> | ||
<div id="frame" className="inverse-color-scheme full-height column-space"> | ||
<ShotPageHeader isOwner={this.props.isOwner} isFxaAuthenticated={this.props.isFxaAuthenticated} | ||
shot={this.props.shot} expireTime={this.props.expireTime} shouldGetFirefox={renderGetFirefox}> | ||
shot={this.props.shot} expireTime={this.props.expireTime} shouldGetFirefox={renderGetFirefox} | ||
staticLink={this.props.staticLink}> | ||
{ favoriteShotButton } | ||
{ editButton } | ||
{ copyButton } | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe staticLink should be defined in the base header, instead of being passed around, since it seems to be used in all the headers
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Slightly confused by above comment, by base header you mean inside header.js, am I missing something here?