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.

Sign up for GitHub

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

Jump to bottom

Add undo trailing icon #3323

Merged
merged 1 commit into from
Oct 21, 2022
Merged

Add undo trailing icon #3323

merged 1 commit into from
Oct 21, 2022

Conversation

Pytal
Copy link
Contributor

@Pytal Pytal commented Sep 30, 2022 •
edited
Loading

Add an "undo" icon in addition to the "close" and "arrowRight" icons

@Pytal Pytal added enhancement New feature or request 3. to review Waiting for reviews feature: input-field Covering the InputField, TextField, ... labels Sep 30, 2022
@Pytal Pytal added this to the 7.0.0 milestone Sep 30, 2022
@Pytal Pytal requested review from CarlSchwan, marcoambrosini and a team September 30, 2022 19:43
@Pytal Pytal self-assigned this Sep 30, 2022
@Pytal Pytal requested review from artonge, skjnldsv and szaimen and removed request for a team September 30, 2022 19:43
@Pytal
Copy link
Contributor Author

Pytal commented Sep 30, 2022

For some reason this affects the NcAppSidebar cypress tests

@artonge
Copy link
Contributor

artonge commented Oct 1, 2022 •
edited
Loading

Can we mark trailingButtonIcon as deprecated so we can remove it in a few major bump?

@raimund-schluessler
Copy link
Contributor

For some reason this affects the NcAppSidebar cypress tests

No idea why, but the color of the favorite icon changed.
Before:
NcAppSidebar vue-subtitle_null-starred_true-compact_true-header_image-secondary_button-editable_false-base

After:
NcAppSidebar vue-subtitle_null-starred_true-compact_true-header_image-secondary_button-editable_false-actual

@raimund-schluessler
Copy link
Contributor

raimund-schluessler commented Oct 1, 2022 •
edited
Loading

Could it be that a change in server now makes the cypress tests fail due to #3139? The cypress tests for a PR that got merged yesterday without issues (https://github.com/nextcloud/nextcloud-vue/actions/runs/3159764008/jobs/5152100405) now also fail. I think it is less than ideal that external changes break the tests for this repository without interaction. There is no way of catching or debugging this.

@skjnldsv @marcoambrosini

@raimund-schluessler raimund-schluessler mentioned this pull request Oct 1, 2022
Closed
@raimund-schluessler
Copy link
Contributor

I created a separate issue for this #3325

@marcoambrosini
Copy link
Contributor

@Pytal We deliberately limited the functionality there to control the design a bit more. What trailing icon do you need there?

@Pytal
Copy link
Contributor Author

Pytal commented Oct 4, 2022

Can we mark trailingButtonIcon as deprecated so we can remove it in a few major bump?

Did not mention deprecating anything?

@Pytal
Copy link
Contributor Author

Pytal commented Oct 4, 2022

@Pytal We deliberately limited the functionality there to control the design a bit more. What trailing icon do you need there?

Would need the undo-variant icon from https://materialdesignicons.com, and had thought of adding another string undo to use this icon but thought that allowing any icon would prevent having to update the component each time a new icon is needed in the future

@Pytal Pytal mentioned this pull request Oct 6, 2022
Merged
6 tasks
@Pytal
Copy link
Contributor Author

Pytal commented Oct 12, 2022

Should we add the undo-variant icon case or allow any custom icon @marcoambrosini?

@jotoeri jotoeri modified the milestones: 7.0.0, 7.0.1 Oct 12, 2022
@Pytal Pytal force-pushed the enh/text-field-material-trailing-icon branch from fd57775 to a0f65ce Compare October 18, 2022 01:37
@marcoambrosini
Copy link
Contributor

Yes I think we should add only undo-variant and proceed slowly keep things under control. cc @jancborchardt @nimishavijay

@Pytal
Add undo trailing icon
7b3a7cf 
Signed-off-by: Christopher Ng <[email protected]>
@Pytal Pytal force-pushed the enh/text-field-material-trailing-icon branch from a0f65ce to 7b3a7cf Compare October 18, 2022 23:48
@Pytal Pytal changed the title Allow use of any trailing material design icon in TextField Add undo trailing icon Oct 18, 2022
@Pytal
Copy link
Contributor Author

Pytal commented Oct 18, 2022

Review-ready

@Pytal Pytal added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Oct 21, 2022
@Pytal Pytal merged commit e27af11 into master Oct 21, 2022
@Pytal Pytal deleted the enh/text-field-material-trailing-icon branch October 21, 2022 02:41
@nickvergessen nickvergessen modified the milestones: 7.0.1, 7.0.2 Oct 25, 2022
@skjnldsv skjnldsv modified the milestones: 7.0.2, 7.1.0 Nov 16, 2022
@Pytal Pytal mentioned this pull request Nov 30, 2023
Merged
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@szaimen szaimen szaimen approved these changes

@marcoambrosini marcoambrosini marcoambrosini approved these changes

@CarlSchwan CarlSchwan Awaiting requested review from CarlSchwan

@artonge artonge Awaiting requested review from artonge

@skjnldsv skjnldsv Awaiting requested review from skjnldsv

Assignees

@Pytal Pytal

Labels
4. to release Ready to be released and/or waiting for tests to finish enhancement New feature or request feature: input-field Covering the InputField, TextField, ...
Projects
None yet
Milestone
7.1.0
Development

Successfully merging this pull request may close these issues.
8 participants
@Pytal @artonge @raimund-schluessler @marcoambrosini @szaimen @nickvergessen @skjnldsv @jotoeri