1172465 - border-radius should be applied to all buttons

Status: RESOLVED WORKSFORME

(Marketplace Graveyard :: Consumer Pages, defect, P5)

Description

[Atique Ahmed Ziad [:atiqueahmedziad]]

Go to - https://marketplace.firefox.com/app/zirma

See the header nav menus, Search button, Register & Sign In buttons, also Free button, they are all having a border-radius. 

But when you scroll down you see the buttons - "Email Support", "Privacy Policy", "Sign in to review", "Read all reviews", "Rating details", "Report Abuse", "Sign me up". These buttons don't have border radius. 

It looks like the upper side buttons have border radius but when I am scrolling the page below, I am seeing the buttons there don't have any border radius. I think we should add 5px border radius to .buttons so that all buttons including "Free" buttons will have same border radius. It may look good !

Comment 1

[Atique Ahmed Ziad [:atiqueahmedziad]]

@Phil: What do you say about this ?

Comment 2

[Philip [:pwalm]]

Hi Atique,

Thanks for the input. We will be adding a border radius to all our buttons in an upcoming design revision of Marketplace later this year.

Comment 3

[Ayush Agrawal]

i want to work on this bug

Comment 4

[shivansh bajaj]

i want to work on it

Comment 5

[Ram Dayal Vaishnav [:ram]]

Assigning this bug to Ayush.

Comment 6

[Ayush Agrawal]

Attachment: Pull Request

Kindly review

Comment 7

[Kevin Ngo [:ngoke]]

Thanks! I cleaned up your branch for you: https://github.com/mozilla/fireplace/pull/1423

Comment 8

[Valentina Virlics [:vvalentina], Ecosystem QA]

Some buttons haven't received a border-radius. :)
Please see the screenshots:
-> "Save Changes" from Settings page: http://screencast.com/t/5sbydKX3E7
-> "Submit Feedback" when accessed from Toggle Settings Menu: http://screencast.com/t/XM5WzBvYR
Verified on MP-dev FF42(Win 7)
Reopening.

Comment 9

[Ayush Agrawal]

Attachment: Pull Request

Comment 10

[Ayush Agrawal]

I had fixed the error.

Kindly review

Comment 11

[Trishul]

Hi Ayush,

(In reply to Ayush Agrawal from comment #10)
> I had fixed the error.
> 
> Kindly review


Try to squash all commits into single commit as following

git rebase -i HEAD~4

Refer 
http://gitready.com/advanced/2009/02/10/squashing-commits-with-rebase.html

Comment 12

[Ayush Agrawal]

Attachment: Pull Request

Corrected errors specified after last merge
Screenshots attached:
https://www.dropbox.com/s/koeiphcieqtjzh6/screenshot1.jpg?dl=0
https://www.dropbox.com/s/1517ssf4i6s7tde/screenshot2.png?dl=0

Kindly Review

Comment 13

[Valentina Virlics [:vvalentina], Ecosystem QA]

I saw, that the changes can be seen on mobile, too.
There are some buttons without border radius on Android 4.2.1 using MP-dev FF42.
Please see the screeshots for:
-> Submit/Update a review buttons
-> Submit a report
-> Sign me up newsletter button
http://screencast.com/t/KfU3o1QT

No issues on FF OS.

Comment 14

[Valentina Virlics [:vvalentina], Ecosystem QA]

The issues from comment 8 are now in production.

Comment 15

[Ayush Agrawal]

Attachment: Pull Request

Fixed The CSS order.
Added border-radius to few more buttons.

Kindly Review

Comment 16

[Atique Ahmed Ziad [:atiqueahmedziad]]

Hi all, 
thanks ValentinaP for the screenshot. I think we should change the border radius of buttons mentioned in comment 13 

And also I think we should not change popup button border radius since the popup window has no border radius. example: report abuse, submit review, feedback popup in desktop. 

I want phil's & kevin's opinion regarding this :)

Comment 17

[Valentina Virlics [:vvalentina], Ecosystem QA]

(In reply to Atique Ahmed Ziad [:atiqueahmedziad] from comment #16)
> Hi all, 
> thanks ValentinaP for the screenshot. I think we should change the border
> radius of buttons mentioned in comment 13 
> 
> And also I think we should not change popup button border radius since the
> popup window has no border radius. example: report abuse, submit review,
> feedback popup in desktop. 
> 
> I want phil's & kevin's opinion regarding this :)

There are two ways to access Feedback on desktop, as you probably know, and one is from Toggle Settings Menu, and is not a popup. I think the "Submit feedback" button from there should have a border-radius.
Please see the screenshot: http://screencast.com/t/GVndtnyzZ

Comment 18

[Atique Ahmed Ziad [:atiqueahmedziad]]

(In reply to ValentinaP from comment #17)
> There are two ways to access Feedback on desktop, as you probably know, and
> one is from Toggle Settings Menu, and is not a popup. I think the "Submit
> feedback" button from there should have a border-radius.
> Please see the screenshot: http://screencast.com/t/GVndtnyzZ

yes, I agree, that should have a border-radius.

Thanks :)

Comment 19

[Valentina Virlics [:vvalentina], Ecosystem QA]

With new desktop-games feature, there is another button: http://screencast.com/t/fEraA7tAZW accessed from game details page. :)

Comment 20

[Mark Striemer [:mstriemer]]

I looks like this was fixed in https://github.com/mozilla/fireplace/pull/1425 but the Report an Issue page is using a `<button>` which isn't caught in our updated styles.

Comment 21

[Ayush Agrawal]

Attachment: Pull Request

Fixed the border-radius of Report an issue & submit feedback buttons
Screenshots Attached
https://www.dropbox.com/s/3fa1aru2inft3dr/Screenshot%20from%202015-08-26%2014%3A24%3A57.png?dl=0
https://www.dropbox.com/s/i1bpwto0em46la3/Screenshot%20from%202015-08-26%2014%3A25%3A17.png?dl=0

Kindly Review

Comment 22

[Ram Dayal Vaishnav [:ram]]

Hi Ayush, did you get chance to work on review comments you received on your PR?

Comment 23

[Ayush Agrawal]

sir i tried it but both the buttons are same so when trying remove border-radius from pop-up window border-radius gets removed from feedback page.

Comment 24

[Trishul]

Hey Ayush, thanks for your effort, please view following comment on PR

https://github.com/mozilla/fireplace/pull/1467#diff-for-comment-39919179

Comment 25

[Ayush Agrawal]

sir i have fixed the border radius.

Kindly Review.

Comment 26

[Atique Ahmed Ziad [:atiqueahmedziad]]

Attachment: setting page.png

the pr- https://github.com/mozilla/fireplace/pull/1467 is still open and I am seeing some unexpected behavior in settings page. There one button contains border-radius & one doesn't.
Ayush, can you have a look ?

Comment 27

[Ayush Agrawal]

Hey Atique, I checked it, the button which you are referring to has already been fixed in my previous PR.
As i have mentioned in Comment 12
May be its because it hasn't been merged yet.

Comment 28

[Trishul]

As per spasovski comment on PR, closing this.

Comment 29

[Valentina Virlics [:vvalentina], Ecosystem QA]

All buttons have border radius, less the "Save changes" button from Settings.
Please see the sccreenshot: http://screencast.com/t/gca5MTsRV
Let's add a border-radius to this button, too. What do you think, Davor Spasovski?

Comment 30

[Davor Spasovski [:spasovski]]

Obviously not very important but it would be more consistent to have the "save changes" button have rounded corners also. Reopen as P5 if you wish.

Comment 31

[Ayush Agrawal]

Hey ValentinaP, as i mentioned it has already been fixed from my side but is yet to be merged in the master

Comment 32

[Davor Spasovski [:spasovski]]

Hi Ayush. That PR isn't really necessary anymore. If you would like to keep working on this we would only need to cover the one remaining case of the "save changes" button but this bug seems quite irrelevant in the big picture and there are more interesting UI issues to handle. If you would like to help out with some of these please feel free to reach out in IRC.