Skip to content

refactor(frontend): Manage tokens modal #5882

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

Merged
merged 48 commits into from
Apr 22, 2025

Conversation

daviddecentage
Copy link
Collaborator

@daviddecentage daviddecentage commented Apr 15, 2025

Motivation

According to new figma designs, we adjust the manage tokens modal.

Changes

  • Updated ManageTokens and ManageTokensModal to use ModalTokensList
  • Added translations
  • Removed testId for removed button, added general close modal testId and adjusted tests

Tests

Bildschirmaufnahme_20250417_111325.mp4

AntonioVentilii and others added 24 commits April 10, 2025 09:36
…factor(frontend)/manage-tokens

# Conflicts:
#	src/frontend/src/lib/components/manage/ManageTokens.svelte
#	src/frontend/src/lib/components/manage/ManageTokensModal.svelte
#	src/frontend/src/lib/components/tokens/ModalTokensList.svelte
…' into refactor(frontend)/manage-tokens

# Conflicts:
#	src/frontend/src/lib/components/tokens/ModalTokensList.svelte
@daviddecentage daviddecentage changed the title refactor(frontend): Manage tokens refactor(frontend): Manage tokens modal Apr 17, 2025
@daviddecentage daviddecentage marked this pull request as ready for review April 17, 2025 09:40
@daviddecentage daviddecentage requested a review from a team as a code owner April 17, 2025 09:40
Copy link
Collaborator

@AntonioVentilii AntonioVentilii left a comment

Choose a reason for hiding this comment

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

To facilitate the review, it would be better to split this PR in smaller scopes, for example, removing the first two elements in two separate PRs, then maybe wrapping in the if condition, or even the usage of ModalTokensList, or the introduction of the the context with the change of the filter logic

@daviddecentage daviddecentage added the run-e2e-snapshots It needs the CI for E2E Snapshots label Apr 17, 2025
@daviddecentage daviddecentage removed the run-e2e-snapshots It needs the CI for E2E Snapshots label Apr 17, 2025
Copy link
Collaborator

@AntonioVentilii AntonioVentilii left a comment

Choose a reason for hiding this comment

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

LGTM tks!

@daviddecentage daviddecentage enabled auto-merge (squash) April 22, 2025 11:43
@daviddecentage daviddecentage merged commit 1e6af74 into main Apr 22, 2025
29 checks passed
@daviddecentage daviddecentage deleted the refactor(frontend)/manage-tokens branch April 22, 2025 12:05
daviddecentage added a commit that referenced this pull request Apr 22, 2025
# Motivation

Adding tests to up coverage so I can merge #5882. The tested component
is used there so it is related.

# Changes

- Adding testIds and tests
- Small fix regarding `<p>` in `<p>` warning

# Tests

Added tests

---------

Co-authored-by: Antonio Ventilii <[email protected]>
Co-authored-by: Max <[email protected]>
Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
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.

3 participants