Skip to content

Conversation

@mejo-
Copy link
Member

@mejo- mejo- commented Dec 7, 2021

Desktop

Full screen:
Screenshot 2021-12-14 at 16-00-27 Keyboard Shortcuts md - Nextcloud

Toolbar on wide screen:
2021-12-14T13:42:15,175191977+01:00

Toolbar on narrow screen:
Screenshot 2021-12-14 at 16-01-13 Keyboard Shortcuts md - Nextcloud

Mobile

Screenshot 2021-12-14 at 16-00-08 Keyboard Shortcuts md - Nextcloud

Toolbar:
Screenshot 2021-12-14 at 15-59-31 Keyboard Shortcuts md - Nextcloud

  • Target version: master

@mejo- mejo- added feature: formatting Features related to text formatting and node types 2. developing labels Dec 7, 2021
@mejo- mejo- force-pushed the enh/help_modal branch 2 times, most recently from d366893 to e933b54 Compare December 9, 2021 16:05
@mejo- mejo- changed the title WIP: Add formatting help modal (Fixes #1941) Add formatting help modal (Fixes #1941) Dec 9, 2021
@mejo-
Copy link
Member Author

mejo- commented Dec 9, 2021

After playing around with examples below the table I finally decided to get rid of them altogether. It's just too much information for such a short overview in a modal and toggling between syntax view and formatted view really doesn't work well for formats like heading1. Especially Cc @nimishavijay

@szaimen
Copy link
Contributor

szaimen commented Dec 9, 2021

How does it look like on very small display sizes? E.g. 360px wide?

@mejo- mejo- force-pushed the enh/help_modal branch 2 times, most recently from 45c82cd to 218e919 Compare December 9, 2021 20:09
@mejo-
Copy link
Member Author

mejo- commented Dec 9, 2021

How does it look like on very small display sizes? E.g. 360px wide?

Good question :) See the updated screenshot in the PR description above. It's not super smooth there, you have to scroll from left to right to read the full syntax.

Do you have a good idea how to improve the mobile experience?

@szaimen
Copy link
Contributor

szaimen commented Dec 9, 2021

Do you have a good idea how to improve the mobile experience?

Maybe hide the keyboard shortcuts column and make the modal full-screen on such small screens?

@mejo-
Copy link
Member Author

mejo- commented Dec 9, 2021

Maybe hide the keyboard shortcuts column and make the modal full-screen on such small screens?

Keyboard shortcuts are already hidden on mobile 😊

And modals alread have full width on mobile per default. The non-used space on both sides seems to come from the invisible prev/next elements. If I override their width to 0, the button to close the modal is no longer visible 😞

@szaimen
Copy link
Contributor

szaimen commented Dec 9, 2021

The non-used space on both sides seems to come from the invisible prev/next elements. If I override their width to 0, the button to close the modal is no longer visible 😞

Strange!
Would maybe the following work?

	.prev,
	.next { display:none !important }

@jancborchardt
Copy link
Member

Looks very nice now! :) Only thing: Would be better to put the button as last item of the fornatting bar instead of next to sharing. That way on narrower viewports it doesnt take a space but gets ellipsized away, as it also isnt always needed. :)

@azul
Copy link
Contributor

azul commented Dec 9, 2021

This looks great! I particularly like the i next to the save indicator as the trigger.

Just a minor note: The description on mobile still talks about keyboard shortcuts.
I'd also remove the 'experience'. 'Speed up your writing with simple shortcuts' is enough i think.

How about using the icons instead of words for the style in mobile? That would also serve as an explaination for the icons. Something like this (with the icons from the menu bar on the left obviously):

style syntax
B **Bold**
I *Italic*
S ~~Strikethrough~~
H1 # largest heading
H6 ###### small heading

This way people would also know it's the same thing as that button.

Signed-off-by: Jonas Meurer <jonas@freesources.org>
@mejo-
Copy link
Member Author

mejo- commented Dec 13, 2021

Looks very nice now! :) Only thing: Would be better to put the button as last item of the fornatting bar instead of next to sharing. That way on narrower viewports it doesnt take a space but gets ellipsized away, as it also isnt always needed. :)

Agreed, done so now:

2021-12-13T18:42:43,344139281+01:00

@szaimen
Copy link
Contributor

szaimen commented Dec 13, 2021

Would maybe the following work?

	.prev,
	.next { display:none !important }

So I suppose this doesn't work either?

@mejo-
Copy link
Member Author

mejo- commented Dec 14, 2021

The non-used space on both sides seems to come from the invisible prev/next elements. If I override their width to 0, the button to close the modal is no longer visible disappointed

Strange! Would maybe the following work?

	.prev,
	.next { display:none !important }

Yep, that indeed works, but as written the button to close the modal is hidden in this case (it's not really hidden, but it's white on white). Also, being able to click into the prev/next areas for closing the modal is no longer possible.

Do you have a good idea how to tackle this? 🤔

@szaimen
Copy link
Contributor

szaimen commented Dec 14, 2021

Do you have a good idea how to tackle this? 🤔

I think it will be already resolved with vue 5.0 due to my changes here: https://github.com/nextcloud/nextcloud-vue/pull/2126/files#diff-907877e92ada6bf9340562928a4046f5a937f4985041a4f2e3981313538f8f5dR679-R697
Maybe this could be of some inspiration?

Signed-off-by: Jonas Meurer <jonas@freesources.org>
@mejo-
Copy link
Member Author

mejo- commented Dec 14, 2021

Do you have a good idea how to tackle this? thinking

I think it will be already resolved with vue 5.0 due to my changes here: https://github.com/nextcloud/nextcloud-vue/pull/2126/files#diff-907877e92ada6bf9340562928a4046f5a937f4985041a4f2e3981313538f8f5dR679-R697 Maybe this could be of some inspiration?

Uh nice, that helped a lot! Thanks @szaimen 😊

I'm now setting position: absolute and top: 50px on mobile, see my last commit: 1048884

I updated the screenshots above.

Signed-off-by: Jonas Meurer <jonas@freesources.org>
@mejo-
Copy link
Member Author

mejo- commented Dec 14, 2021

This looks great! I particularly like the i next to the save indicator as the trigger.

Just a minor note: The description on mobile still talks about keyboard shortcuts. I'd also remove the 'experience'. 'Speed up your writing with simple shortcuts' is enough i think.

How about using the icons instead of words for the style in mobile? That would also serve as an explaination for the icons. Something like this (with the icons from the menu bar on the left obviously):
style syntax
B **Bold**
I *Italic*
S ~~Strikethrough~~
H1 # largest heading
H6 ###### small heading

This way people would also know it's the same thing as that button.

Nice idea! I gave it a try, but found the written style ("Bold", ...) to be much clearer. Now that all content fits on mobile screens (at least for english), I'd prefer to keep it that way.

Here's how it looked with the buttons:

Screenshot 2021-12-14 at 13-51-13 Keyboard Shortcuts md - Nextcloud

Signed-off-by: Jonas Meurer <jonas@freesources.org>
Signed-off-by: Jonas Meurer <jonas@freesources.org>
Copy link
Member

@jancborchardt jancborchardt left a comment

Choose a reason for hiding this comment

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

Except the "Show formatting help" → "Formatting help" as mentioned @mejo- this is super nice work design-wise! :)

Copy link
Member

@juliusknorr juliusknorr left a comment

Choose a reason for hiding this comment

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

Very nice 👍 Good to go from my side

Signed-off-by: Jonas Meurer <jonas@freesources.org>
@mejo- mejo- merged commit 6bf0184 into master Dec 15, 2021
@delete-merged-branch delete-merged-branch bot deleted the enh/help_modal branch December 15, 2021 09:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review feature: formatting Features related to text formatting and node types

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants