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

Responsive design for mobile #457

Closed
wants to merge 7 commits into from

Conversation

Projects
None yet
3 participants
@pom2ter
Copy link
Contributor

commented Jan 20, 2016

Here is my take for issue #222 , feedback will be needed.
Basically the roster pane is partially hidden on mobile and a hamburger icon appear at the top right to open / close the pane when clicked.

I added the dependencies (bootstrap) with bower and a couple with grunt (css minification).
CSS changes were made in the default.css file, at the bottom.
As for javascript, i tried to keep the modification to a minimum. What I added is probably not at the best place but it works on my end ;)

@benlangfeld

This comment has been minimized.

Copy link
Member

commented Jan 20, 2016

Looks sweet. Would you mind dropping some representative screenshots here, @pom2ter ?

@pom2ter

This comment has been minimized.

Copy link
Contributor Author

commented Jan 21, 2016

This was taken with the Xcode simulator on Mac. it represents the IPhone 5. The roster pane is not completely hidden, enough to show the number of occupants. messages in chat are all align to the left because of the small screen.

simulator screen shot 20 janv 2016 20 30 53

When the roster pane is opened by clicking/tapping the hamburger icon on the top right, it takes the same space as default. The icon is done in css as i didnt want to add images.

simulator screen shot 20 janv 2016 20 31 00

@marclaporte

This comment has been minimized.

Copy link

commented Jan 31, 2016

@benlangfeld So what do you think?

Would you be up for running a community demo with this code?

Thanks!

@benlangfeld

This comment has been minimized.

Copy link
Member

commented Jan 31, 2016

So this is working fine for me. A few requests before I can merge this; would you please

  1. follow the existing CSS style of inserting a blank space between a property name and its value;
  2. provide some more thorough comments on the various blocks of CSS rules to describe their purpose a little more clearly than "tweaking stuff";
  3. remove commented lines in CSS;
  4. remove blocks of > 1 blank lines between CSS rules;
  5. edit existing styles rather than overriding them in the same CSS file.

Thanks for the work on this, it's much appreciated!

@pom2ter

This comment has been minimized.

Copy link
Contributor Author

commented Feb 1, 2016

ok i made the changes

@benlangfeld

This comment has been minimized.

Copy link
Member

commented Feb 1, 2016

Looks good, would you squash for merge @pom2ter?

@pom2ter

This comment has been minimized.

Copy link
Contributor Author

commented Feb 1, 2016

Im not sure if it worked, never use squash before.

@benlangfeld

This comment has been minimized.

Copy link
Member

commented Feb 1, 2016

Thanks @pom2ter !

@pom2ter pom2ter deleted the pom2ter:responsive-design branch Feb 24, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.