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

Change archive back icon #4217

Merged
merged 3 commits into from
Oct 17, 2024
Merged

Change archive back icon #4217

merged 3 commits into from
Oct 17, 2024

Conversation

nicodh
Copy link
Contributor

@nicodh nicodh commented Oct 16, 2024

resolve #4184

@nicodh nicodh requested a review from WofWca October 16, 2024 05:59
Copy link
Collaborator

@WofWca WofWca left a comment

Choose a reason for hiding this comment

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

image

Copy link
Member

@r10s r10s left a comment

Choose a reason for hiding this comment

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

very nice to push that forward! that will be a big usability improvement!

currently, only the color is wrong - this is visible also in dark mode, but even more in light mode:

Screenshot 2024-10-16 at 11 07 41

the arrow should just get the color of the text. this would also improve the display in settings (i assume that is the origin of <backButton>): also in settings, the grey looks a bit disabled. just using text color would also better there

@r10s
Copy link
Member

r10s commented Oct 16, 2024

i just noticed, that we have the arrow already in the correct layout when the chatlist is hidden:

Screenshot 2024-10-16 at 15 55 29

maybe better reuse that?

@nicodh
Copy link
Contributor Author

nicodh commented Oct 16, 2024

Icon color is now the same as text color

@r10s
Copy link
Member

r10s commented Oct 16, 2024

Icon color is now the same as text color

it picks up body-text-color, but not title-bar-text-color 😅 (which are maybe same in dark mode, but not in lite mode) (sorry if i have been unclear above)

Screenshot 2024-10-16 at 16 35 44

@nicodh
Copy link
Contributor Author

nicodh commented Oct 16, 2024

Icon color is now the same as text color

it picks up body-text-color, but not title-bar-text-color 😅 (which are maybe same in dark mode, but not in lite mode)
(sorry if i have been unclear above)

Oh my gosh! Sorry is on my side! - I only checked it in the settings dialog, so it seems we have to declare a special prop to differentiate if we are on a dialog header or a navbar.

Seems too easy...
I will have a look tomorrow

@WofWca WofWca self-requested a review October 16, 2024 19:02
@nicodh
Copy link
Contributor Author

nicodh commented Oct 16, 2024

I now reuse the back button from small screen

No idea why it was not used also on default screen size before

Copy link
Member

@r10s r10s left a comment

Choose a reason for hiding this comment

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

very nice!

cannot say much to the css, but it works and looks as expected now :)

@Simon-Laux Simon-Laux merged commit c53d95c into main Oct 17, 2024
10 checks passed
@Simon-Laux Simon-Laux deleted the archive-back-button branch October 17, 2024 02:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

use standard "Back" button for archive
4 participants