-
Notifications
You must be signed in to change notification settings - Fork 119
6257 use buttons instead of links #7087
base: master
Are you sure you want to change the base?
6257 use buttons instead of links #7087
Conversation
ea36e4c
to
54cc73c
Compare
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.
Thanks for this contribution @AlbionaHoti, you are well on your way to having this merged. I left a few inline comments, and am leaving a couple more:
- The original bug refers to the links in the avatar change modal, can you add the styles there too?
- Please squash your commits into a single commit so that when merged, the git history is cleaner.
@@ -138,6 +138,15 @@ | |||
margin-top: 25px; | |||
} | |||
|
|||
.save-option-button { | |||
background: none !important; |
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.
Try to not use !important
unless absolutely necessary, it makes the assumption that we'll never want to override this style. It's far preferable is to use a more specific selector. See https://css-tricks.com/when-using-important-is-the-right-choice/ for reference.
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.
@@ -138,6 +138,15 @@ | |||
margin-top: 25px; | |||
} | |||
|
|||
.save-option-button { |
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.
If we add these styles to the avatar change modal as specified in the original issue, these styles will need to be extracted into a shared location. I suggest somewhere in _button-row.scss.
style(buttons): use buttons instead of links style(buttons): use buttons in recovery_key file refactor(style): remove unnecessary style refactor(style): remove unnecessary style refactor(style): remove unnecessary style refactor(style): remove unnecessary style refactor(style): remove unnecessary style refactor(style): remove unnecessary style refactor(style): remove unnecessary style refactor(style): remove unnecessary style refactor(style): remove unnecessary style refactor(style): remove unnecessary style refactor(style): remove unnecessary style
refactor(settings): order properties refactor(settings): order properties refactor(settings): order properties refactor(settings): order properties
2266053
to
6e0ea9b
Compare
This repo has been deprecated and migrated to https://github.com/mozill/fxa. Please open this PR against that repo. |
Hi @lmorchard, can you please review this issue?
fixes #6257