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

compose: Move row of formatting buttons into the textbox. #29953

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

N-Shar-ma
Copy link
Collaborator

@N-Shar-ma N-Shar-ma commented May 4, 2024

The row of buttons is placed using CSS grid template areas so that visually it is now inside the bottom edge of the textbox. The color of the buttons row and individual buttons is changed to match the color of the textbox. All textbox border / box shadow properties are now applied to its parent instead which is extended under the buttons' row, so that its border snuggly fits around the buttons row too.

Notable side effects:

  • In dark mode the textbox in focused state now has a light border which does not match the recipient input's current border which doesn't change when focused. Likely, the recipient input should be updated to match the textbox's border color.
  • The dividers in the formatting buttons row are not vertically centered now. This should be figured out soon.

Known concern:

  • The textbox resize handle still shows, breaking the illusion of the buttons row being inside the textbox. It will be removed when enabling a 3 stage resizeable textbox.

Fixes: #28702.

Screenshots and screen captures:

Light Mode
image
Hovering a button:
image
Focus in textbox:
image
Preview mode:
image

Dark Mode
image
Hovering a button:
image
Focus in textbox:
image
Preview mode:
image

Self-review checklist
  • Self-reviewed the changes for clarity and maintainability
    (variable names, code reuse, readability, etc.).

Communicate decisions, questions, and potential concerns.

  • Explains differences from previous plans (e.g., issue description).
  • Highlights technical choices and bugs encountered.
  • Calls out remaining decisions and concerns.
  • Automated tests verify logic where appropriate.

Individual commits are ready for review (see commit discipline).

  • Each commit is a coherent idea.
  • Commit message(s) explain reasoning and motivation for changes.

Completed manual review and testing of the following:

  • Visual appearance of the changes.
  • Responsiveness and internationalization.
  • Strings and tooltips.
  • End-to-end functionality of buttons, interactions and flows.
  • Corner cases, error conditions, and easily imagined bugs.

@zulipbot zulipbot added size: S area: compose (UI & widgets) Scheduled messages, giphy, send later, etc. redesign Part of the webapp redesign project, including blockers. labels May 4, 2024
@zulipbot
Copy link
Member

zulipbot commented May 4, 2024

Hello @zulip/design members, this pull request was labeled with the "redesign" label, so you may want to check it out!

@N-Shar-ma N-Shar-ma force-pushed the btns-in-textbox branch 2 times, most recently from cc66ddd to 5a68023 Compare May 18, 2024 08:33
@zulipbot zulipbot added size: L and removed size: M labels May 18, 2024
@N-Shar-ma N-Shar-ma force-pushed the btns-in-textbox branch 3 times, most recently from 0f4be6d to 2c4c700 Compare May 20, 2024 06:24
@N-Shar-ma N-Shar-ma marked this pull request as ready for review May 20, 2024 06:32
@alya
Copy link
Contributor

alya commented May 20, 2024

Cool! A couple of notes from testing the PR:

  1. This change breaks keyboard navigation when you use Tab starting from the main compose area. The focus goes from the send buttons to the top of app, rather than the formatting buttons.
  2. The hover color between the buttons and the three-dot buttons menu is inconsistent in dark theme. I guess we should remove the blue in the menu?
    Screenshot 2024-05-20 at 09 41 59
  3. When playing with the width, the ? button overflows.
    Screenshot 2024-05-20 at 09 35 56

Screenshot 2024-05-20 at 09 35 41

That's all I've got so far!

@N-Shar-ma
Copy link
Collaborator Author

N-Shar-ma commented May 21, 2024

@alya Thanks for the feedback!

  1. Even after lots of investigation, I'm not sure why the buttons are skipped when tabbing. I'll soon make a #development-help thread to figure this out (https://chat.zulip.org/#narrow/stream/101-design/topic/Changing.20tabbing.20order)
  2. Thanks for noticing this issue! Fixed:
    image
  3. This problem was only on Chrome (not Firefox) since the native scrollbar there takes up a significant amount of space, making the compose box narrower. I've tweaked the breakpoints to fit in, in Chrome too

The row of buttons is placed using CSS grid template areas so that
visually it is now inside the bottom edge of the textbox. The color of
the buttons row and individual buttons is changed to match the color of
the textbox. All textbox border / box shadow properties are now applied
to its parent instead which is extended under the buttons' row, so that
its border snuggly fits around the buttons row too.

Notable side effects:
- In dark mode the textbox in focused state now has a light border which
does not match the recipient input's current border which doesn't change
when focused. Likely, the recipient input should be updated to match the
textbox's border color.
- The dividers in the formatting buttons row are not vertically centered
now. This should be figured out soon.

Known concern:
- The textbox resize handle still shows, breaking the illusion of the
buttons row being inside the textbox. It will be removed when enabling
a 3 stage resizeable textbox.

Fixes: zulip#28702.
@N-Shar-ma
Copy link
Collaborator Author

@alya The tabbing order has been fixed! All feedback so far has now been addressed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: compose (UI & widgets) Scheduled messages, giphy, send later, etc. redesign Part of the webapp redesign project, including blockers. size: XL
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move compose box buttons into text area
3 participants