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.

Sign up for GitHub

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

Jump to bottom

[#588] CodeView: align the checkboxes #589

Merged
merged 21 commits into from Mar 22, 2019
Merged

[#588] CodeView: align the checkboxes #589

merged 21 commits into from Mar 22, 2019

Conversation

fzdy1914
Copy link
Member

@fzdy1914 fzdy1914 commented Mar 6, 2019 •
edited by yamidark

Fixes #588

After #532 , the name of the checkbox is changed from mui-checkbox to
mui-checkbox--filetype, causing an alignment problem with the
checkboxes.

Since this rename is meaningful, let's add another css part to ensure
that the checkboxes align.

Meanwhile, some of the indentation in #532 does not follow our
standard, let's update them as well.

@fzdy1914 fzdy1914 changed the title [585] Align the checkboxes in code view [#585] Align the checkboxes in code view Mar 6, 2019
@fzdy1914 fzdy1914 requested a review from a team March 6, 2019 16:51
chel-seyy
chel-seyy reviewed Mar 7, 2019
View reviewed changes
Copy link
Contributor

@chel-seyy chel-seyy left a comment

Choose a reason for hiding this comment

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

image
Is it possible to align the next row's checkboxes with the previous row as well?

frontend/src/static/css/style.scss Outdated Show resolved Hide resolved
@fzdy1914 fzdy1914 changed the title [#585] Align the checkboxes in code view [#588] Align the checkboxes in code view Mar 7, 2019
ongspxm
ongspxm reviewed Mar 7, 2019
View reviewed changes
Copy link
Contributor

@ongspxm ongspxm left a comment

Choose a reason for hiding this comment

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

all those indentations hahhah

frontend/src/static/css/style.scss Show resolved Hide resolved
@fzdy1914 fzdy1914 requested a review from chel-seyy March 7, 2019 06:56
chel-seyy
chel-seyy approved these changes Mar 7, 2019
View reviewed changes
Copy link
Contributor

@chel-seyy chel-seyy left a comment

Choose a reason for hiding this comment

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

Looks better now 👍

@fzdy1914 fzdy1914 requested a review from a team March 7, 2019 14:06
@yamidark yamidark requested a review from eugenepeh March 7, 2019 14:25
@fzdy1914 fzdy1914 requested a review from eugenepeh March 9, 2019 16:01
eugenepeh
eugenepeh reviewed Mar 9, 2019
View reviewed changes
Copy link
Member

@eugenepeh eugenepeh left a comment

Choose a reason for hiding this comment

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

👍 Looks better.

However since you have create a div for the file types, can you correspondingly do the same for the search box? This is to standardize the code.

Another one more thing, can you make both the radio buttons flushed to the top? Because it seems that it is flushed to the top for the check boxes but not for the search bar.

image

@fzdy1914
Copy link
Member Author

Another one more thing, can you make both the radio buttons flushed to the top? Because it seems that it is flushed to the top for the check boxes but not for the search bar.

Have made it flushed to top. But it is a bit weird now when the checkboxes only have one line.

@fzdy1914 fzdy1914 requested a review from eugenepeh March 10, 2019 16:48
@fzdy1914 fzdy1914 closed this Mar 12, 2019
@fzdy1914 fzdy1914 reopened this Mar 12, 2019
@yamidark yamidark requested review from ongspxm and eugenepeh and removed request for eugenepeh March 13, 2019 08:30
@eugenepeh eugenepeh requested a review from ongspxm March 16, 2019 14:47
@yamidark yamidark requested review from ongspxm and removed request for ongspxm March 16, 2019 19:48
ongspxm
ongspxm approved these changes Mar 17, 2019
View reviewed changes
Copy link
Contributor

@ongspxm ongspxm left a comment

Choose a reason for hiding this comment

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

👍

@yamidark
Copy link
Contributor

@fzdy1914 Please resolve the conflicts with latest master

@emer7 emer7 mentioned this pull request Mar 20, 2019
Merged
@fzdy1914
Merge branch 'master' into align
fe64b7d 
# Conflicts:
#	frontend/src/index.jade
@fzdy1914
Copy link
Member Author

Due to the request in #445 (comment), I will not solve the problem in this pr.

@fzdy1914 fzdy1914 requested a review from yamidark March 20, 2019 12:51
@yamidark yamidark requested review from damithc and removed request for damithc March 21, 2019 06:11
@yamidark yamidark changed the title [#588] Align the checkboxes in code view [#588] CodeView: align the checkboxes Mar 22, 2019
@yamidark yamidark merged commit 9b3ebfd into reposense:master Mar 22, 2019
@fzdy1914 fzdy1914 deleted the align branch March 22, 2019 13:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ongspxm ongspxm ongspxm approved these changes

@damithc damithc damithc approved these changes

@yamidark yamidark yamidark approved these changes

@eugenepeh eugenepeh eugenepeh approved these changes

@chel-seyy chel-seyy chel-seyy approved these changes

@yong24s yong24s Awaiting requested review from yong24s

Assignees
No one assigned
Labels
s.ToMerge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@fzdy1914 @eugenepeh @yamidark @ongspxm @damithc @chel-seyy