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

Feat/adjust sks menu #479

Merged
merged 6 commits into from
Dec 10, 2024
Merged

Feat/adjust sks menu #479

merged 6 commits into from
Dec 10, 2024

Conversation

simon-the-shark
Copy link
Member

  1. small UI tweaks - consulted with @odominiak but still quite opinionated - you can tell me what you think. I did hovewer improve responsiveness a bit
  2. I added "technical notes" implementation and note about historic menu.

STILL TODO: Caching and pull to refresh

previously

image

now

image
image

WhatsApp.Video.2024-12-09.at.22.05.28.mp4

technical notes

image

@simon-the-shark simon-the-shark added the enhancement New feature or request label Dec 9, 2024
@simon-the-shark simon-the-shark self-assigned this Dec 9, 2024
@simon-the-shark simon-the-shark linked an issue Dec 9, 2024 that may be closed by this pull request
feat/adjust sks and add technical notes impl #480
Closed
mikolaj-jalocha
mikolaj-jalocha reviewed Dec 10, 2024
View reviewed changes
Copy link
Member

@mikolaj-jalocha mikolaj-jalocha left a comment

Choose a reason for hiding this comment

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

Good work here! I like the visual changes. Left few comments/suggestions/questions of mine :-)

lib/features/sks-menu/data/models/sks_menu_response.dart Show resolved Hide resolved
lib/features/sks-menu/presentation/sks_menu_screen.dart Show resolved Hide resolved
lib/features/sks-menu/presentation/widgets/technical_message.dart Outdated Show resolved Hide resolved
lib/features/sks-menu/presentation/widgets/technical_message.dart Show resolved Hide resolved
lib/widgets/my_text_button.dart Outdated Show resolved Hide resolved
@mikolaj-jalocha
Copy link
Member

do we want to show te counter if sks's closed?

@tomasz-trela
Copy link
Member

I like this ui changes

tomasz-trela
tomasz-trela previously approved these changes Dec 10, 2024
View reviewed changes
Copy link
Member

@tomasz-trela tomasz-trela left a comment

Choose a reason for hiding this comment

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

After applying some of Mikołaj's changes LGTM!

@simon-the-shark simon-the-shark merged commit d92bf9e into main Dec 10, 2024
2 checks passed
@simon-the-shark simon-the-shark deleted the feat/adjust-sks-menu branch December 10, 2024 18:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mikolaj-jalocha mikolaj-jalocha mikolaj-jalocha left review comments

@tomasz-trela tomasz-trela tomasz-trela left review comments

Assignees

@simon-the-shark simon-the-shark

Labels
enhancement New feature or request
Projects
None yet
Milestone
v1.2.0 - Mr. Shark, We Have to Cook
Development

Successfully merging this pull request may close these issues.

feat/adjust sks and add technical notes impl
3 participants
@simon-the-shark @mikolaj-jalocha @tomasz-trela