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

Add image editor #1317

Merged
merged 3 commits into from
Aug 17, 2022
Merged

Add image editor #1317

merged 3 commits into from
Aug 17, 2022

Conversation

skjnldsv
Copy link
Member

@skjnldsv skjnldsv commented Aug 9, 2022

image

Design review and todo

Overall

  • Edit would be nicer directly in the top right, outside of 3-dot menu. (Needs change in component, but would also be generally nice to have more actions outside of that 3-dot menu like download and "Open sidebar")
  • Edit icon can be simple pencil ("edit") from Material Icons
  • Background currently white, a bit jarring on the switch, better to use dark/black background so it’s similar to the Viewer background (but opaque). Not influenced by theming, always dark, like Viewer
    See Allow to manually specify a theme to apply to a section of an app server#33564
  • Default tab: Adjust. Default tool: Crop (always first thing in both lists)
  • Current primary color is purple from the library, would be nice if this can come from theming
  • Loading spinner is non-standard, we should use ours instead
  • Possibly remove ".jpeg" option in save dialog as simple ".jpg" is already there
  • When any dropdown is open, there’s a scrollbar on the right side of screen → Leads to interface moving around
  • Is the list of tools on the left accessible?
    • Ideally use our proper text color: color-main-text
    • Our text-size: 15px (navigation sizes need to be increased a bit)
  • Border-radius
    • Tools on left: border-radius-large
    • Tool details on bottom: border-radius
  • Would be nice if keyboard shortcuts work:
    • Ctrl+S for saving
    • Ctrl+Z for undo

Actions

  • Header bar of Viewer is not needed, can be overlapped
  • Reset button would be better with text "Reset" instead of icon, as it’s a very destructive action
    • When pressing reset, dialog with "Continue" action, action label would be better "Reset changes".
    • Also color would be better red than orange -> Not possible
  • Pressing close X button when changes are present should bring up an "Unsaved changes" dialog
    • Same for pressing Escape
  • Buttons like undo/redo, zoom in/out, and close could be tertiary icon-only buttons
  • "Side by side" better wording "View original"

Adjust

  • "Original" dropdown has large height which leads to layout issues

Resize

  • Lock button would look better icon-only

Watermark

  • Should be hidden as it’s the same as Draw (via Text and Image)

For later

  • Also are you sure that using a different name or file extension will always work? What about cases where you are only allowed to edit the file but not save in the target directory?
  • Is the list of tools on the left accessible?
  • Edit would be nicer directly in the top right, outside of 3-dot menu. (Needs change in component, but would also be generally nice to have more actions outside of that 3-dot menu like download and "Open sidebar")
  • Use dark theme
  • Disable when mobile
  • on a publlic page does the auto-generated path look a bit strange and the dropdown is missing some spacing on the left?

@skjnldsv skjnldsv added enhancement New feature or request 2. developing Work in progress labels Aug 9, 2022
@skjnldsv skjnldsv self-assigned this Aug 9, 2022
@skjnldsv skjnldsv added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Aug 16, 2022
@skjnldsv skjnldsv requested review from a team, PVince81, artonge and Pytal and removed request for a team August 16, 2022 14:52
@skjnldsv skjnldsv added this to the Nextcloud 25 milestone Aug 16, 2022
@skjnldsv

This comment was marked as resolved.

Signed-off-by: John Molakvoæ <skjnldsv@protonmail.com>
@szaimen

This comment was marked as resolved.

@skjnldsv
Copy link
Member Author

skjnldsv commented Aug 16, 2022

Nice!
I guess I will fix it in a followup then.
I'm not even sure it's usable on mobile, Need to check with the UX team.

Some other issues to fix after:

  • The viewer doesn't refresh the saved image
  • The preview also doesn't seems to update

EDIT: Image saving is fixed

Signed-off-by: John Molakvoæ <skjnldsv@protonmail.com>
@szaimen

This comment was marked as resolved.

@skjnldsv
Copy link
Member Author

  • Also are you sure that using a different name or file extension will always work? What about cases where you are only allowed to edit the file but not save in the target directory?

Good call! I need to have a look at that option too.

  • text is too dark?

Will fix small design in this PR.
Big other changes will be split for easier reviews :)

@skjnldsv
Copy link
Member Author

Fixed
image

Signed-off-by: John Molakvoæ <skjnldsv@protonmail.com>
@szaimen
Copy link
Contributor

szaimen commented Aug 17, 2022

Two small things remaning:

  • on a publlic page does the auto-generated path look a bit strange and the dropdown is missing some spacing on the left?
    image

Copy link
Contributor

@szaimen szaimen left a comment

Choose a reason for hiding this comment

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

LGTM in my testing when the follow-ups get addressed.
But did not really review the code.

@szaimen
Copy link
Contributor

szaimen commented Aug 21, 2022

Do we want to add the remaining points to an overview ticket for easier discoverability? (Searching for this PR is not as easy imho)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants