Closed Bug 1172465 Opened 9 years ago Closed 8 years ago

border-radius should be applied to all buttons

Categories

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

Avenir
defect

Tracking

(Not tracked)

RESOLVED WORKSFORME
2015-08-11

People

(Reporter: atiqueahmedziad, Assigned: ayushagrawal288, Mentored)

Details

(Whiteboard: [ktlo], [good first bug])

Attachments

(6 files)

46 bytes, text/x-github-pull-request
Details | Review
46 bytes, text/x-github-pull-request
Details | Review
46 bytes, text/x-github-pull-request
Details | Review
46 bytes, text/x-github-pull-request
Details | Review
46 bytes, text/x-github-pull-request
Details | Review
26.02 KB, image/png
Details
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 !
@Phil: What do you say about this ?
Flags: needinfo?(pwalmsley)
Severity: normal → trivial
Priority: -- → P5
Whiteboard: [ktlo]
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.
Flags: needinfo?(pwalmsley)
Whiteboard: [ktlo] → [ktlo], [good first bug]
i want to work on this bug
i want to work on it
Flags: needinfo?(vaishnav.rd)
Flags: needinfo?(trishul.goel)
Assigning this bug to Ayush.
Assignee: nobody → ayushagrawal288
Status: NEW → ASSIGNED
Flags: needinfo?(vaishnav.rd)
Flags: needinfo?(trishul.goel)
Attached file Pull Request
Kindly review
Thanks! I cleaned up your branch for you: https://github.com/mozilla/fireplace/pull/1423
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2015-08-11
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.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attached file Pull Request
I had fixed the error.

Kindly review
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
Attached file 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
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.
The issues from comment 8 are now in production.
Attached file Pull Request
Fixed The CSS order.
Added border-radius to few more buttons.

Kindly Review
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 :)
Flags: needinfo?(pwalmsley)
Flags: needinfo?(kngo)
(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
(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 :)
With new desktop-games feature, there is another button: http://screencast.com/t/fEraA7tAZW accessed from game details page. :)
Mentor: trishul.goel
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.
Hi Ayush, did you get chance to work on review comments you received on your PR?
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.
Flags: needinfo?(trishul.goel)
Hey Ayush, thanks for your effort, please view following comment on PR

https://github.com/mozilla/fireplace/pull/1467#diff-for-comment-39919179
Flags: needinfo?(trishul.goel)
sir i have fixed the border radius.

Kindly Review.
Status: REOPENED → RESOLVED
Closed: 9 years ago9 years ago
Flags: needinfo?(pwalmsley)
Resolution: --- → WONTFIX
Attached image 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 ?
Flags: needinfo?(kngo) → needinfo?(ayushagrawal288)
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
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.
Flags: needinfo?(ayushagrawal288)
As per spasovski comment on PR, closing this.
Status: REOPENED → RESOLVED
Closed: 9 years ago8 years ago
Resolution: --- → WORKSFORME
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?
Flags: needinfo?(dspasovski)
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.
Flags: needinfo?(dspasovski)
Hey ValentinaP, as i mentioned it has already been fixed from my side but is yet to be merged in the master
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.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: