-
Notifications
You must be signed in to change notification settings - Fork 295
add text to email account menu #344
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
Conversation
|
Added a screenshot above, now easier to review. :) |
|
Easy review, please check this out @nextcloud/mail :) |
| deleteButton: 'button[class^="icon-delete"]', | ||
| settingsButton: 'button[class^="icon-rename"]' | ||
| menu: '.app-navigation-entry-menu', | ||
| settingsButton: '.action-settings', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
doesn't this change mean that clicking the text has no effect? Only the icon will have a click handler registered now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope, because .action-settings is the whole a element which spans the full width. ;)
js/templates/account.html
Outdated
| </div> | ||
| {{/if}} | ||
|
|
||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unnecessary line breaks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed :)
|
@jancborchardt ping 😉 |
Signed-off-by: Jan-Christoph Borchardt <hey@jancborchardt.net>
b1dfe11 to
996776a
Compare
|
Addressed your concerns and rebased to master. :) |
ChristophWurst
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CI failure seems unrelated -> can be merged IMO
|
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs and questions. |
Check out the 3-dot menu of the mail account :) now has text next to the icons like all other menus in Nextcloud.

Please review @nextcloud/mail @nextcloud/designers