Improve[UI]: Responsive footer, overlapping of footer and div in contribute page and buttons spacing on collection page for better UI #597
Conversation
…xed the overlapping of footer
…xed the overlapping of footer
|
Thanks for opening your first pull request for BookBrainz, and welcome to our community! 🎉 If you haven't yet, please check out our contributing guidelines. Someone will be reviewing your PR soon; just hang in there ! |
trishnakalita660
changed the title
trishnakalita660
changed the title
There was a problem hiding this comment.
Thanks for opening a pull request @trishnakalita660 !
I've made some suggestions below, mainly to use existing CSS classes for margins instead of the style attribute.
This allows for a bit more control in CSS, since the rules applied in an element's style take precedence over the css defined in stylesheets:
From https://developer.mozilla.org/en-US/docs/Web/CSS/Specificity:
Inline styles added to an element (e.g., style="font-weight: bold;") always overwrite any styles in external stylesheets, and thus can be thought of as having the highest specificity.
There are also a couple of files which shouldn't have been commited: latest.sql and package-lock.json
| @@ -34,7 +34,7 @@ function ContributePage() { | |||
| 'en/latest/style/introduction/'; | |||
|
|
|||
| return ( | |||
| <div> | |||
| <div styles={{marginBottom: '1rem'}}> | |||
There was a problem hiding this comment.
| <div styles={{marginBottom: '1rem'}}> | |
| <div className="margin-bottom-1"> |
| @@ -311,6 +317,7 @@ class CollectionPage extends React.Component { | |||
| bsSize="small" | |||
| bsStyle="danger" | |||
| className="margin-bottom-d5" | |||
| style={{marginBottom: '1rem', marginRight: '0.5rem'}} | |||
There was a problem hiding this comment.
Let's remove the inline styles and change the className="margin-bottom-d5" above to `className="margin-bottom-1 margin-right-d5"
| { | ||
| this.props.isCollaborator || this.props.isOwner ? | ||
| <Button | ||
| bsSize="small" | ||
| bsStyle="success" | ||
| className="margin-bottom-d5" | ||
| style={{marginRight: '0.5rem'}} |
There was a problem hiding this comment.
Let's remove this and add margin-right-d5 to the className above
| className="margin-top-1 text-left" | ||
| style={{marginBottom: '1rem'}} |
There was a problem hiding this comment.
| className="margin-top-1 text-left" | |
| style={{marginBottom: '1rem'}} | |
| className="margin-top-1 margin-bottom-1 text-left" |
There was a problem hiding this comment.
Sorry it took a while to get to reviewing again!
We're nearly there, I've got a couple more suggestions, and we need to remove the changes to .gitignore and package-lock.json that don't need to be in there.
You can run this command in a terminal, from the bookbrainz-site directory:
git checkout origin/master -- .gitignore package-lock.json
What that does is revert these two files to the content of the master branch.
You can then commit the changes to these two files, and that should do it to remove them from the PR.
Let me know if you run into issues or need some help!
| href={`/collection/${this.props.collection.id}/edit`} | ||
| style={{marginRight: '0.5rem'}} |
There was a problem hiding this comment.
The style can be removed, since we're using the class margin-right-d5 instead.
| disabled={!this.state.selectedEntities.length} | ||
| style={{marginRight: '0.5rem'}} |
There was a problem hiding this comment.
The style can be removed, since we're using the class margin-right-d5 instead.
| @@ -283,8 +284,9 @@ class CollectionPage extends React.Component { | |||
| <Button | |||
| bsSize="small" | |||
| bsStyle="danger" | |||
| className="margin-bottom-d5" | |||
| className="margin-right-d5" | |||
There was a problem hiding this comment.
| className="margin-right-d5" | |
| className="margin-bottom-d5 margin-right-d5" |
| @@ -298,8 +300,9 @@ class CollectionPage extends React.Component { | |||
| <Button | |||
| bsSize="small" | |||
| bsStyle="warning" | |||
| className="margin-bottom-d5" | |||
| className="margin-right-d5" | |||
There was a problem hiding this comment.
| className="margin-right-d5" | |
| className="margin-bottom-d5 margin-right-d5" |
|
Closed in favor of #739, as we are harmonizing the footers between all MetaBrainz projects |
Problem
Improve[UI]: Responsive footer, fixed overlapping of footer and div in contribute page and added left margin to buttons on collection page for better UI
Solution
Fixed the size and bootstrap col configurations in the footer and Added margins for the buttons on collection page.
Areas of Impact
-> bookbrainz-site\src\client\components\pages\collection.js
-> bookbrainz-site\src\client\components\footer.js
-> bookbrainz-site\src\client\components\pages\contribute.js