Skip to content

Kieran gould/12hourtimestamp#3961

Merged
ara4n merged 4 commits intoelement-hq:developfrom
Kieran-Gould:kieran-gould/12hourtimestamp
May 26, 2017
Merged

Kieran gould/12hourtimestamp#3961
ara4n merged 4 commits intoelement-hq:developfrom
Kieran-Gould:kieran-gould/12hourtimestamp

Conversation

@Kieran-Gould
Copy link
Copy Markdown
Contributor

@Kieran-Gould Kieran-Gould changed the base branch from master to develop May 18, 2017 21:24
.mx_EventTile_selected .mx_EventTile_line {
border-left: $accent-color 5px solid;
padding-left: 60px;
padding-left: 100px;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

please let's not change the whole layout of the app just to provide the option of 12h timestamps ;)

See comments of https://github.com/matrix-org/matrix-react-sdk/pull/903/files#r117365391

{ DateUtils.formatTime(date) }
</span>
);
if (UserSettingsStore.getSyncedSetting('showTwelveHourTimestamps')) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

technically the state of render() should depend only on props & state rather than js-sdk state, but i'm happy to ignore this for now :)

</span>
);
}
else {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

else should be on previous line for readability :)

</span>
);
if (UserSettingsStore.getSyncedSetting('showTwelveHourTimestamps')) {
return (
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

indentation should be 4 spaces pleaseeeee

.mx_FilePanel .mx_EventTile_selected .mx_EventTile_line {
padding-left: 0px;
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

probably not worth messing up latest commit on this file for this

}

.mx_EventTile_e2eIcon_12hr {
padding-left: 5px;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

indentation pl0x

@ara4n
Copy link
Copy Markdown
Member

ara4n commented May 18, 2017

this lgtm other than concerns on the react-sdk one (including how to structure the CSS)

@Kieran-Gould Kieran-Gould force-pushed the kieran-gould/12hourtimestamp branch from 3343de7 to cae62c8 Compare May 19, 2017 21:36
@ara4n
Copy link
Copy Markdown
Member

ara4n commented May 26, 2017

lgtm having fixed typo, i think

@ara4n ara4n merged commit 9156a87 into element-hq:develop May 26, 2017
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.

3 participants