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

Comment field fixes #40173

Closed
wants to merge 2 commits into from
Closed

Conversation

Jerome-Herbinet
Copy link
Member

@Jerome-Herbinet Jerome-Herbinet commented Aug 31, 2023

Respawn of #38501 (avoids conflicts of old PR and more complete fix)

Testing from reviewers needed.

Fixes :

  • Field's height when empty
  • Submit button's location

Before

Capture.video.du.31-08-2023.10.51.47.webm

After

Capture.video.du.31-08-2023.10.52.10.webm

Checklist

Signed-off-by: Jérôme Herbinet <33763786+Jerome-Herbinet@users.noreply.github.com>
Signed-off-by: Jérôme Herbinet <33763786+Jerome-Herbinet@users.noreply.github.com>
Copy link
Member

@skjnldsv skjnldsv left a comment

Choose a reason for hiding this comment

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

I think that require a fix on the vue component directly
Setting a position: relative; on the .rich-contenteditable__input seems to do the trick and wrap the placeholder.

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.

Visually looks good to me, but it would be best to use variables instead of fixed values here, based on var(--default-grid-baseline) (4px).

@Jerome-Herbinet a quick general note regarding reopening pull requests – it's always nicer to keep it in the existing pull request (stuff can be force pushed or rebased for example), because otherwise reviews are spread among multiple pull requests. And if you do absolutely need to do a new PR, make sure to delete the branch of the old one. :)

@Jerome-Herbinet
Copy link
Member Author

Visually looks good to me, but it would be best to use variables instead of fixed values here, based on var(--default-grid-baseline) (4px).

@Jerome-Herbinet a quick general note regarding reopening pull requests – it's always nicer to keep it in the existing pull request (stuff can be force pushed or rebased for example), because otherwise reviews are spread among multiple pull requests. And if you do absolutely need to do a new PR, make sure to delete the branch of the old one. :)

OK @jancborchardt I understand, and I'll delete all my closed PRs' branches.

@skjnldsv skjnldsv mentioned this pull request Nov 1, 2023
This was referenced Nov 6, 2023
This was referenced Nov 14, 2023
@blizzz blizzz modified the milestones: Nextcloud 28, Nextcloud 29 Nov 23, 2023
@skjnldsv
Copy link
Member

Fixed with 28 and the new activity/comments merge

@skjnldsv skjnldsv closed this Feb 24, 2024
@skjnldsv skjnldsv removed the 3. to review Waiting for reviews label Feb 24, 2024
@skjnldsv skjnldsv deleted the Jerome-Herbinet-comment-field-fixes branch February 24, 2024 16:28
@skjnldsv skjnldsv removed this from the Nextcloud 29 milestone Aug 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants