1185832 - [Review] Dates should align with the 'Report' link

Status: RESOLVED FIXED

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

Description

[Atique Ahmed Ziad [:atiqueahmedziad]]

Attachment: bug.png

STP: 
1. Go to - https://marketplace-dev.allizom.org/app/exquisite-barbeque-7/ratings
2. Have a look at the dates

Actual Result: 
the dates are not align with the 'Report' link
 
Expected Result: 
The dates should align with the 'Report' link

Comment 1

[Atique Ahmed Ziad [:atiqueahmedziad]]

I am not sure whether its by design. But aligning the dates with 'Report',  make sense & looks good. 
https://www.dropbox.com/s/7hp9k77q7aspiom/Screen%20Shot%202015-07-03%20at%2002.27.09.png?dl=0
@Philip, can you confirm it ? :)

Comment 2

[Kevin Ngo [:ngoke]]

Aligning it makes sense.

Comment 3

[Atique Ahmed Ziad [:atiqueahmedziad]]

Thanks,  leaving the bugs to newbies :)

Comment 4

[rahultalreja26051994]

Hey all, i am a newbie to code base contribution area and i would like to work over this bug under mentorship of Atique !

Comment 5

[Atique Ahmed Ziad [:atiqueahmedziad]]

Thanks for taking the bug :) 
The bug belongs to the front-end part of marketplace i.e. Fireplace 
If you have a github account, can you please fork the fireplace repository from github?  
Fireplace repository: https://github.com/mozilla/fireplace

After forking give a reply :)

I have question, do you use windows or linux?  :)

Comment 6

[Atique Ahmed Ziad [:atiqueahmedziad]]

If you have any question,  please don't hesitate to ask, mate :)

Comment 7

[rahultalreja26051994]

Hey Atique i am a windows user and i do have a github account and a client on my desktop too, ill fork and let u know.
https://github.com/rahultalreja

Comment 8

[Atique Ahmed Ziad [:atiqueahmedziad]]

It is complex to setup fireplace in your localhost from windows.(I couldn't setup in windows after trying thousand time) So I will suggest you to install Linux (Ubuntu) in your pc. It's easy to setup fireplace locally in linux :)
Thanks

Comment 9

[Andreas Wagner [:TheOne] [use NI]]

Are you still working on this? Did you manage to setup your environment? Can we help?

Comment 10

[rahultalreja26051994]

Hey Andreas,
This is the first bug for me so i am bit slow with the process as i am learning so many new things along with contributing also the new platform is quite different so i am not so again it takes time for me. Also for last few days i wasnt keeping well so was off track but now i am back and i have started working on it.
Yes i am done with environment setup, i will update with next steps by Monday.
Thanks and please cooperate.

Comment 11

[rahultalreja26051994]

Hey all , i am very sorry for being so late !!
please consider as i am a very newbie to the contribution tract and the platform used (linux OS), so it tooked to get hands onn, also initially i did silly mistakes like removing GRUB of my primary OS(windows 8.1) and formating one of my disk partition where i had important data !!
i would surely write a blog post on all my experiences for the newbies who would start to contribute on bugs in future. 
P.S. i have done with initial set on my local server i would work on code part and update here asap!
Thanks

Comment 12

[Atique Ahmed Ziad [:atiqueahmedziad]]

Hi, 
A big congrats on installing Fireplace in your localhost. 
Now have a look at this file - https://github.com/mozilla/fireplace/blob/master/src/templates/_macros/review.html#L45-L46
I think if we take the time element inside the </ul> in line 45, then it should align with "Report"
Give a try changing it & let me know if it works. :)

Comment 13

[rahultalreja26051994]

sure, work in process, 
will update soon .
i guess by changing following code(line 304 on review.styl (fireplace/src/media/css/) sheet also would do:
*Initially 
review time {
        +ltr() {
            right: 20px;
        }
        +rtl() {
            left: 20px;
        }
    }
*Finally
review time {
         bottom: 0px;
        +ltr() {
            right: 20px;
        }
        +rtl() {
            left: 20px;
        }
    }

Comment 14

[Atique Ahmed Ziad [:atiqueahmedziad]]

Hi, Great :D that will work also, 
please put the `bottom:0;` before the `position:absolute;` here- https://github.com/mozilla/fireplace/blob/4b7dd2651b47f9b867c0a6aafb63e96bad257a51/src/media/css/reviews.styl#L35-L36

Thanks , give a pull request, and flag for review :)

Comment 15

[Mark Striemer [:mstriemer]]

Attachment: mock.png

The review date should be on its own line below the comment and above the report links.

Comment 16

[Atique Ahmed Ziad [:atiqueahmedziad]]

Great,  thanks mark :) 
Rahul, have a look at the design & make the change in code according to the design. 

If you need any help, let me know.

Comment 17

[Atique Ahmed Ziad [:atiqueahmedziad]]

A follow up, are you still working on it, Rahul ?

Comment 18

[rahultalreja26051994]

hey Atique, 
i went through design and i will submit my request by the end of the day!
i am sorry was little bit occupied due to festival here.

Comment 19

[rahultalreja26051994]

Attachment: Pull Request

Kindly review PR.

Comment 20

[David Durst [:ddurst]]

Any progress on this? If this is still unchanged in two weeks, we'll unassign it. (I see that there is feedback on the PR, so you can update the PR accordingly.)

Comment 21

[Atique Ahmed Ziad [:atiqueahmedziad]]

the pr- https://github.com/mozilla/fireplace/pull/1576 is merged.

Congratulation to Rahul for fixing it. Thanks!

Comment 22

[Atique Ahmed Ziad [:atiqueahmedziad]]

https://github.com/mozilla/fireplace/commit/a3c29c7332a0264e714341dadaf7acd4ae75225e