Update: Add copy to clipboard for about brave page (fixes #5790) #6107
Conversation
| @@ -82,6 +82,9 @@ body { | |||
| font-size: 16px; | |||
| margin-bottom: 12px; | |||
| padding-left: @aboutPageSectionPadding; | |||
| width: 400px; | |||
| display: flex; | |||
| justify-content: space-between; | |||
There was a problem hiding this comment.
I think maybe we should move these three styles to parent selector of .sectionTitle
so the original sectionTitle style will remain the same.
Present look:
Expected look:
There was a problem hiding this comment.
And if we do so, width: 424px would be aligned
There was a problem hiding this comment.
adding margin-right to .aboutAbout .fa-clipboard would work too.
There was a problem hiding this comment.
Is the expectation, that we want to move the placement of icon slightly to the left?
There was a problem hiding this comment.
Sorry, I didn't put the icon aligned. What I meant is
<div className='title'>
<span className='sectionTitle' data-l10n-id='versionInformation' />
<span className='fa fa-clipboard' title='Copy password to clipboard' onClick={this.onCopy} />
</div>
We wanna keep the original attribute for version information like orange color.
So we can put the attributes of display and justify-content into .title
And as @luixxiul suggest, we can add margin-right to .fa-clipboard
There was a problem hiding this comment.
ah i see now. Sorry abt that I dint realize we lost the color of the title. Will make the change tonight.
| @@ -30,7 +40,10 @@ class AboutBrave extends React.Component { | |||
| </div> | |||
|
|
|||
| <div className='siteDetailsPageContent aboutAbout'> | |||
| <div className='sectionTitle' data-l10n-id='versionInformation' /> | |||
| <div className='sectionTitle'> | |||
| <span data-l10n-id='versionInformation' /> | |||
There was a problem hiding this comment.
Please see the comment in less/about/siteDetails.less
|
@darkdh I made all the changes mentioned in the comments. Let me know if anything else is needed. |
|
@gyandeeps , great job, thanks! |
| width: 400px; | ||
| display: flex; | ||
| justify-content: space-between; | ||
| margin-left: @aboutPageSectionPadding; |
There was a problem hiding this comment.
Should this not be margin-right?
There was a problem hiding this comment.
That is the same margin-left aligned with the sortable table below
|
Please re-submit a PR once fixed. Just wanted to also mention I don't think we should be special casing this about page's section title for consistency reasons. |
|
I will work on it with cherry-pick |
Closes #6183 Redo PR #6107 (which was reverted with #6184) - cherry-picked 942e95c - Added brave.less for about:brave to fix regression on about pages TODO: Pick up properties from history.less into brave.less Auditors: @alexwykoff Test Plan: 1. Visit about:about
git rebase -ito squash commits (if needed).Test Plan:
This is my first PR. I wasn't sure what sort test needs to be written for copy to clipboard functionality.
UI
Same as mentioned in the issue #5790 (comment)
Copied data to clipboard: