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

fix(frontend/settings): change theme tool button position #586

Closed
wants to merge 4 commits into from

Conversation

saihaj
Copy link
Member

@saihaj saihaj commented Jun 30, 2020

Summary of PR

Changes position of overlay button in settings

Before
image

After
image

Time spent on PR

2 mins

Related

#583 (comment)

Reviewers

@bhajneet @Harjot1Singh

@bhajneet
Copy link
Member

bhajneet commented Jul 5, 2020

This should not have been opened for review with such a noticeable style issue.

image

@bhajneet
Copy link
Member

bhajneet commented Jul 5, 2020

Possibly related to #519

@saihaj
Copy link
Member Author

saihaj commented Jul 6, 2020

@Harjot1Singh should I address #519 in new PR or do it here? @bhajneet is the button layout change acceptable though?

@Harjot1Singh
Copy link
Member

It might actually make more sense to do #519 first if it won’t take too long, and merge it in

@saihaj
Copy link
Member Author

saihaj commented Aug 25, 2020

@bhajneet MaterialUI Grid and Button have some sort of padding is causing the green lines as shown in your screenshot.

image

@bhajneet
Copy link
Member

Can you add a css rule for sibling of button to remove it's top padding?

fixes linting errors
@saihaj
Copy link
Member Author

saihaj commented Aug 25, 2020

Removing top padding doesn't help. I also tried margin but that didn't work.
image

That yellow region is the one that needs to be removed somehow.

image

@bhajneet
Copy link
Member

Yeah, so that would be top margin then. Rule would be similar to

.MuiButtonbase-root + .MuiButtonbase-root {
  margin-top: 0;
}

@Harjot1Singh
Copy link
Member

What is happening with this PR @saihaj?

@saihaj
Copy link
Member Author

saihaj commented Oct 25, 2020

I think me and @bhajneet were trying to do something and that didn’t work. Can you please run it locally and see. I think this is what the issue is #586 (comment)

@bhajneet bhajneet assigned bhajneet and unassigned saihaj Oct 28, 2020
@bhajneet bhajneet added Status: WIP Effort 1 Simple task (code/non-code). and removed Status: To Do labels Oct 28, 2020
@bhajneet bhajneet added the Impacts Few Does not affect many end-users. label Oct 28, 2020
@bhajneet bhajneet added ⊖ Blocked ⊖ Not moving for some reason. Refine prioritization! and removed Status: WIP labels Oct 28, 2020
@bhajneet
Copy link
Member

Too stale

@bhajneet bhajneet closed this Oct 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⊖ Blocked ⊖ Not moving for some reason. Refine prioritization! Effort 1 Simple task (code/non-code). Impacts Few Does not affect many end-users.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants