Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Changed style of enter grade link #68

Merged
merged 1 commit into from
Mar 27, 2015

Conversation

amir-qayyum-khan
Copy link

Changed enter grade link style to make it look like button.
#15

@carsongee
Copy link

For UI changes, it would be handy to have a screen shot with the PR in the future.

@carsongee
Copy link

Since the goal is to make this look like the other button, is there any reason you didn't just switch this to an input button and use the same class as the annotated button? Also, as is the size is slightly off between the two:
image

@amir-qayyum-khan
Copy link
Author

Hi

  1. Annotated button use class="fileupload" and with this class we trigger file upload function
  2. Annotated button code is

    Upload annotated file

Input is placed inside button, i dont now why?

  1. Enter grade use href="#{{ id }}-enter-grade" to open dialog

@amir-qayyum-khan
Copy link
Author

size of button
fixed the size of button

@amir-qayyum-khan amir-qayyum-khan force-pushed the chage_enter_grade_style branch from 722787a to a4cea4b Compare March 17, 2015 11:09
@carsongee
Copy link

Thanks for the explanation. I think we should then create a common class for both i.e. .sga-block staff-button and have both those change to use the new class. Please also switch the Enter Grade a tag to an input button so they have consistent HTML as well

@amir-qayyum-khan
Copy link
Author

@carsongee I am working on this PR

@amir-qayyum-khan amir-qayyum-khan force-pushed the chage_enter_grade_style branch from 7659c9a to 8956a7f Compare March 19, 2015 11:29
@pdpinch
Copy link
Member

pdpinch commented Mar 19, 2015

@amir-qayyum-arbisoft I just noticed you pushed a bunch of commits with the same commit message. That makes it really hard to figure out what you're doing. I know that sometimes you can end up with more commits than you intended -- in those situations you can squash them (edX has docs on this; I frequently use this guide myself)

@amir-qayyum-khan
Copy link
Author

@pdpinch i will gona sqash these commits, just testing different scenarios, thanks for these documents

@amir-qayyum-khan amir-qayyum-khan force-pushed the chage_enter_grade_style branch 5 times, most recently from a504458 to a38eb92 Compare March 19, 2015 14:35
{% trans "Enter grade" %}
<% } %>
</a>
<a class="enter-grade-button button" href="#{{ id }}-enter-grade">

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: two space index

@carsongee
Copy link

There is still a slight size difference:
image
but once that is fixed and merge conflicts are resolved I'm 👍

@amir-qayyum-khan amir-qayyum-khan force-pushed the chage_enter_grade_style branch 2 times, most recently from e7c06ce to 2f0d436 Compare March 24, 2015 08:14
@amir-qayyum-khan
Copy link
Author

@carsongee done with changes and rebase

color: #333346;
font-weight: bold;
font-size: 0.8125em;
margin-top: 1px;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes the button one pixel lower than the other button. I think that if you remove this line this is definitely good to go. Sorry to be so particular:
image

@amir-qayyum-khan amir-qayyum-khan force-pushed the chage_enter_grade_style branch from c125331 to 00db211 Compare March 27, 2015 11:57
@amir-qayyum-khan
Copy link
Author

@carsongee done with 1px fix
enter_grade_link_fix

@carsongee
Copy link

Awesome, thanks so much @amir-qayyum-arbisoft 🎇

carsongee added a commit that referenced this pull request Mar 27, 2015
@carsongee carsongee merged commit f6a6792 into mitodl:master Mar 27, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants