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

Adapt to different device widths #401

Merged
merged 20 commits into from
Apr 24, 2022
Merged

Adapt to different device widths #401

merged 20 commits into from
Apr 24, 2022

Conversation

C0ffeeCode
Copy link
Contributor

@C0ffeeCode C0ffeeCode commented Mar 18, 2022 •
edited by github-actions bot

Description

Cinny should be more usable on smaller devices or smaller windows.
Therefore, it would be useful in those cases to hide the people drawer since it only takes away too much screen and at some point, hide either the navigation or room view.
The people list can still be accessed on the room settings.
If the UI is separated by nav and room view, a button appears on a room header to go back.

UI on 946px width:
image
UI on 890px width:
image
UI on 400px width on a room:
image
UI on 400px width on navigation:
image

Relates to #55

What does this not address? (yet)

This PR could be merged before the following points are finished.

  • Tab in room settings (will not be changed)
    • replacing scroll view might be considerable
  • Settings do not adapt
  • Important UI sometimes gets cut away (to be fixed in another PR)
    • image
  • Context menus do not fit (to be fixed in another PR)
    • image

Type of change

  • New feature (non-breaking change which adds functionality)

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
    • fix bug: Emoji picker does not appear when width is smaller than 360px

Preview: https://625fcf0d375e065d3faff09a--pr-cinny.netlify.app
⚠️ Exercise caution. Use test accounts. ⚠️

@C0ffeeCode
Now adapting to small screen sizes, needs improvements
3e1ed98
@C0ffeeCode
Fix that site only gets into mobile mode when resized
86940de
@C0ffeeCode
Merge pull from dev
d50f81d
@C0ffeeCode
- Added navigation event triggered if user requests to return to navi…
60c73e3 
…gation on compact screens

- People drawer wont be shown on compact screens
  - Still accessible using settings
  - would be duplicated UI
- mobileSize is now compactSize
@C0ffeeCode
Put threshold for collapsing the base UI in a shared file
47ad308
@C0ffeeCode
Switch to a more simple solution using CSS media queries over JS
ac73a59 
- Move back button to the left a bit so it doesnt get in touch with room icon
@C0ffeeCode
switch from component-individual-thresholds to device-type thresholds
efb7ec7 
- <750px: Mobile
- <900px: Tablet
- >900px: Desktop
@iambumblehead
Copy link

@C0ffeeCode the screenshots look awesome. I look forward to using your changes :)

@ajbura
Copy link
Member

ajbura commented Mar 20, 2022

Hi @C0ffeeCode, I am doing some design changes in user setting panel, so I'd say wait on that so you don't have to do extra work.

@C0ffeeCode
Copy link
Contributor Author

C0ffeeCode commented Mar 20, 2022 •
edited

Ok. 👍
Does this also include changes to space settings and the TitleWrapper on the Header?
image

@ajbura
Copy link
Member

ajbura commented Mar 20, 2022

No, only in user settings.

@C0ffeeCode C0ffeeCode marked this pull request as ready for review March 21, 2022 16:01
@C0ffeeCode C0ffeeCode mentioned this pull request Mar 22, 2022
Closed
7 tasks
ajbura
ajbura reviewed Mar 23, 2022
View reviewed changes
src/app/organisms/room/Room.jsx Outdated Show resolved Hide resolved
src/app/organisms/room/Room.scss Outdated Show resolved Hide resolved
src/app/organisms/room/RoomViewHeader.jsx Outdated Show resolved Hide resolved
src/app/organisms/room/RoomViewHeader.jsx Outdated Show resolved Hide resolved
src/app/partials/threshold.scss Outdated Show resolved Hide resolved
src/app/templates/client/Client.jsx Outdated Show resolved Hide resolved
src/app/templates/client/Client.jsx Outdated Show resolved Hide resolved
src/app/templates/client/Client.jsx Outdated Show resolved Hide resolved
src/client/state/cons.js Outdated Show resolved Hide resolved
src/app/templates/client/Client.jsx Outdated Show resolved Hide resolved
@C0ffeeCode
Improve code quality; allow passing classNames to IconButton
1d69c0f 
- remove unnessesary div wrappers
- use dir.side where appropriate
- rename threshold and its mixins to more descriptive names
- Rename "OPEN_NAVIGATION" to "NAVIGATION_OPENED"
@C0ffeeCode C0ffeeCode requested a review from ajbura March 26, 2022 10:27
@nitanmarcel
Copy link
Contributor

Using this PR as a base for an ubuntu touch app. Nicely done.

One thing that's missing is the 3-dots menu on the room list views which shows outside the view.

Also touch doesn't send an hover event which bugs the message context menu (reply, edit) so probably that needs to be handled in mobile view too somehow.

Looking forward on this PR :)

image

ajbura
ajbura reviewed Mar 29, 2022
View reviewed changes
src/app/organisms/room/RoomViewHeader.jsx Outdated Show resolved Hide resolved
src/app/organisms/room/RoomViewHeader.jsx Outdated Show resolved Hide resolved
src/app/templates/client/Client.jsx Outdated Show resolved Hide resolved
src/client/state/cons.js Outdated Show resolved Hide resolved
src/app/templates/client/Client.jsx Outdated Show resolved Hide resolved
@C0ffeeCode
Copy link
Contributor Author

Which BEM classNames would you propose?

@ajbura
Copy link
Member

ajbura commented Mar 29, 2022

Commented above in review comments.

@thebiblelover7
Copy link

Looking forward for this PR to be merged! Would love to use cinny on my phone!

C0ffeeCode and others added 5 commits April 3, 2022 18:23
@C0ffeeCode
- follow BEM methology
9c31777 
- remove ROOM_SELECTED listener
- rename NAVIGATION_OPENED to OPEN_NAVIGATION where appropriate
- this does NOT changes that ref should be used for changing visability
@C0ffeeCode
Use ref to change visability to avoid re-rendering
70e5a9c
@ajbura
Merge branch 'dev' into adapt-mobile-width
41d7eac
@C0ffeeCode
Use ref to change visability to avoid re-rendering
75ed082
@C0ffeeCode
Fix that room component is not hidden by default.
cb707e5 
This resulted in a broken view when application is viewed in mobile size without having selected a room since loading.
@C0ffeeCode C0ffeeCode requested a review from ajbura April 15, 2022 21:03
ajbura
ajbura approved these changes Apr 24, 2022
View reviewed changes
Copy link
Member

@ajbura ajbura left a comment

Choose a reason for hiding this comment

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

Amazing work. Thanks :)

@ajbura ajbura merged commit dc8e6e5 into cinnyapp:dev Apr 24, 2022
@kfiven kfiven added this to the v2.0.0 milestone Apr 24, 2022
@nitanmarcel
Copy link
Contributor

Awesome. I can base my UT app on the upstream now.

Thanks for merging this

@C0ffeeCode C0ffeeCode deleted the adapt-mobile-width branch April 24, 2022 11:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ajbura ajbura ajbura approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v2.0.0
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@C0ffeeCode @iambumblehead @ajbura @nitanmarcel @thebiblelover7 @kfiven