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

[RSN-34] - Create navbar and footer #40

Merged
merged 8 commits into from
Jun 7, 2024
Merged

[RSN-34] - Create navbar and footer #40

merged 8 commits into from
Jun 7, 2024

Conversation

rogacky11
Copy link
Collaborator

@rogacky11 rogacky11 commented May 18, 2024

Description

I have added visual changes to our web application using tailwind css.

Motivation

This change is needed to create navbar and footer.

Related issue and/or ticket

RSN-34

@rogacky11 rogacky11 added scope: frontend Indicates that PR relates to frontend changes. type: feature PR's categorised as feature. labels May 18, 2024
@rogacky11 rogacky11 requested a review from wzarek May 18, 2024 10:29
@rogacky11 rogacky11 self-assigned this May 18, 2024
@wzarek
Copy link
Owner

wzarek commented May 19, 2024

image

@rogacky11 why there is a link to RSN-14?

  • if it's ready for review why isn't it marked as ready for review?

@rogacky11 rogacky11 added the status: ready for review Determines that PR is ready for review. label May 19, 2024
@rogacky11
Copy link
Collaborator Author

image

@rogacky11 why there is a link to RSN-14?

* if it's ready for review why isn't it marked as ready for review?

I apologize for my mistake, link changed. I also forgot about ready for review status, so it has been immediately added.

Copy link
Owner

@wzarek wzarek left a comment

Choose a reason for hiding this comment

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

Also - better idea would be to put the links - Reasn, logowanie, rejestracja inside another div like below:
image
Instead of making some weird margins mx-60 you can specify it's with in % (3/4 would be 75% as in design).
One more thing - you didn't format it properly from what I see, so it will be needed to use some sort of prettier or something for that.
Also, you can name it not Nav.tsx but something more meaningful like Navbar or Menu or something like that and place it in 'components' folder. Also - making separate components for Links like logowanie and rejestracja could be a good idea, since it will be reusable then and we (for logged user) want to display more than that, so it shouldnt be hardcoded. + if Link type is not used, it should be deleted (also styles import).

And the last thing - where is the footer?

Client/reasn-client/apps/web/app/Nav.tsx Outdated Show resolved Hide resolved
Client/reasn-client/apps/web/app/Nav.tsx Outdated Show resolved Hide resolved
Client/reasn-client/apps/web/app/Nav.tsx Outdated Show resolved Hide resolved
Client/reasn-client/apps/web/app/Nav.tsx Outdated Show resolved Hide resolved
@wzarek wzarek added status: require changes Determines that PR requires changes to be approved. and removed status: ready for review Determines that PR is ready for review. labels May 19, 2024
@wzarek
Copy link
Owner

wzarek commented May 19, 2024

image
@rogacky11 why there is a link to RSN-14?

* if it's ready for review why isn't it marked as ready for review?

I apologize for my mistake, link changed. I also forgot about ready for review status, so it has been immediately added.

Do it via youtrack, just drag the issue in in review state and it will assign the status automatically here

@wzarek wzarek marked this pull request as draft May 20, 2024 16:36
@wzarek wzarek removed the status: require changes Determines that PR requires changes to be approved. label May 20, 2024
@rogacky11 rogacky11 requested a review from wzarek June 1, 2024 15:57
@rogacky11
Copy link
Collaborator Author

rogacky11 commented Jun 1, 2024

Also - better idea would be to put the links - Reasn, logowanie, rejestracja inside another div like below:

Instead of making some weird margins mx-60 you can specify it's with in % (3/4 would be 75% as in design). One more thing - you didn't format it properly from what I see, so it will be needed to use some sort of prettier or something for that. Also, you can name it not Nav.tsx but something more meaningful like Navbar or Menu or something like that and place it in 'components' folder. Also - making separate components for Links like logowanie and rejestracja could be a good idea, since it will be reusable then and we (for logged user) want to display more than that, so it shouldnt be hardcoded. + if Link type is not used, it should be deleted (also styles import).

And the last thing - where is the footer?

I took your advices into account and added adequately.
But still using eg. w-3/4 I think I have to use margin. Am i right?
And the last thinkg - footer has been added.

@rogacky11 rogacky11 added the status: ready for review Determines that PR is ready for review. label Jun 2, 2024
@wzarek
Copy link
Owner

wzarek commented Jun 3, 2024

@rogacky11 if it's ready for review mark it as ready for review, currently it's a draft PR

@wzarek wzarek added status: require changes Determines that PR requires changes to be approved. and removed status: ready for review Determines that PR is ready for review. labels Jun 3, 2024
@rogacky11 rogacky11 marked this pull request as ready for review June 3, 2024 16:31
@wzarek wzarek added status: ready for review Determines that PR is ready for review. and removed status: require changes Determines that PR requires changes to be approved. labels Jun 3, 2024
@wzarek
Copy link
Owner

wzarek commented Jun 5, 2024

@rogacky11 please resolve conflicts / rebase + format code

@wzarek
Copy link
Owner

wzarek commented Jun 6, 2024

image

@wzarek wzarek changed the title RSN-34: Create navbar and footer [RSN-34] - Create navbar and footer Jun 6, 2024
@wzarek
Copy link
Owner

wzarek commented Jun 6, 2024

Created #47

@wzarek wzarek requested review from raczu and wzarek and removed request for wzarek June 6, 2024 19:15
@wzarek wzarek merged commit 4511ced into main Jun 7, 2024
7 checks passed
@wzarek wzarek deleted the RSN-34 branch June 7, 2024 14:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scope: frontend Indicates that PR relates to frontend changes. status: ready for review Determines that PR is ready for review. type: feature PR's categorised as feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants