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(ember): added navigation to navbar #40

Merged
merged 1 commit into from
Aug 21, 2023
Merged

feat(ember): added navigation to navbar #40

merged 1 commit into from
Aug 21, 2023

Conversation

c0rydoras
Copy link
Collaborator

@c0rydoras c0rydoras commented Aug 4, 2023 •
edited
Loading

image
image

@c0rydoras c0rydoras requested a review from anehx August 4, 2023 11:30
@anehx
Copy link
Member

anehx commented Aug 4, 2023

Can you add a screenshot?

@c0rydoras
Copy link
Collaborator Author

@anehx done

anehx
anehx requested changes Aug 4, 2023
View reviewed changes
Copy link
Member

@anehx anehx left a comment

Choose a reason for hiding this comment

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

Why did you choose to use a button group for navigation instead of using the UIkit navigation elements we use in all other applications?

ember/app/components/top-nav/template.hbs Outdated Show resolved Hide resolved
ember/app/styles/components/_top-nav.scss Outdated Show resolved Hide resolved
@c0rydoras c0rydoras force-pushed the ember/navigation branch 3 times, most recently from 0a8dd4a to d5f5edd Compare August 7, 2023 07:11
@anehx
Copy link
Member

anehx commented Aug 7, 2023

Why did you choose to use a button group for navigation instead of using the UIkit navigation elements we use in all other applications?

@c0rydoras could you answer this question?

@c0rydoras
Copy link
Collaborator Author

c0rydoras commented Aug 7, 2023 •
edited
Loading

Why did you choose to use a button group for navigation instead of using the UIkit navigation elements we use in all other applications?

I personally prefer how it looks, and it was done with uk-button-group in previous versions of the app

implementation with uk-subnav
image
current implementation
image

@c0rydoras c0rydoras linked an issue Aug 15, 2023 that may be closed by this pull request
Navigation #57
Closed
anehx
anehx requested changes Aug 21, 2023
View reviewed changes
ember/app/styles/components/_top-nav.scss Outdated Show resolved Hide resolved
@c0rydoras c0rydoras merged commit 595ac2e into adfinis:main Aug 21, 2023
4 checks passed
@c0rydoras c0rydoras deleted the ember/navigation branch August 21, 2023 12:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@anehx anehx anehx approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Navigation
2 participants
@c0rydoras @anehx