Skip to content

[Fix #56] Fixed statistics mobile view for some resolution#61

Merged
CuriousLearner merged 5 commits intopythonindia:masterfrom
pradeepgangwar:gh-pages
Aug 1, 2017
Merged

[Fix #56] Fixed statistics mobile view for some resolution#61
CuriousLearner merged 5 commits intopythonindia:masterfrom
pradeepgangwar:gh-pages

Conversation

@pradeepgangwar
Copy link
Copy Markdown
Contributor

Issue No. - #56
Link to view Changes - Gh-Pages

Changes Made:

  • Added media CSS query for particular resolution as bootstrap grid didn't solved the problem even with full implementation.

Copy link
Copy Markdown
Member

@CuriousLearner CuriousLearner left a comment

Choose a reason for hiding this comment

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

Can you please use default bootstrap classes?

Also, please avoid inline styling.

@pradeepgangwar
Copy link
Copy Markdown
Contributor Author

pradeepgangwar commented Jul 17, 2017

@CuriousLearner In my merged PR #30 . I Implemented changes for mobile view using only bootstrap for all resolutions possible. But #56 issue was raised. Actually problem was only between resolution 480px to 569px, so I decided to change it using media queries which will not affect other classes on website.
I removed inline styles.

@CuriousLearner
Copy link
Copy Markdown
Member

Breakpoints eventually cause trouble when the code base size increases. Can you please use bootstrap grid instead?

@pradeepgangwar
Copy link
Copy Markdown
Contributor Author

@CuriousLearner I have fixed it now with bootstrap grid only. The problem was caused by a supportive CSS file that had such rules for that resolution. The issue looks fixed now.

Comment thread css/main.css Outdated

.hr-statistics{
color: white;
} No newline at end of file
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Leave a line at the end of the file.

Copy link
Copy Markdown
Contributor Author

@pradeepgangwar pradeepgangwar Jul 22, 2017

Choose a reason for hiding this comment

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

@CuriousLearner done. But I don't know why it's not showing here.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@pradeepgangwar your new commit has been added to this PR when you pushed. The review you are seeing is showing the code at the moment of review and it'll remain same.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@realslimshanky fine. @CuriousLearner please review the PR with new changes.

@jasonbraganza
Copy link
Copy Markdown

Can be closed now

@pradeepgangwar
Copy link
Copy Markdown
Contributor Author

@CuriousLearner Please see this. I think it can be merged now.

@sayanchowdhury
Copy link
Copy Markdown
Member

LGTM 🍫

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants