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

Support sub-navigation position end #9873

Merged
merged 17 commits into from
Nov 27, 2023

Conversation

glennjacobs
Copy link
Contributor

@glennjacobs glennjacobs commented Nov 25, 2023

Allows sub-navigation to align-right.

LEFT
image

RIGHT
image

@glennjacobs glennjacobs marked this pull request as ready for review November 25, 2023 22:38
@danharrin danharrin added the enhancement New feature or request label Nov 25, 2023
@danharrin danharrin added this to the v3 milestone Nov 25, 2023
@danharrin danharrin self-assigned this Nov 25, 2023
@glennjacobs glennjacobs marked this pull request as draft November 25, 2023 22:45
@glennjacobs
Copy link
Contributor Author

@danharrin do you think we need the option to set this at the resource level also?

@danharrin
Copy link
Member

Yes, I think so

@glennjacobs glennjacobs marked this pull request as ready for review November 25, 2023 23:13
@glennjacobs
Copy link
Contributor Author

Yes, I think so

Done and added docs.

@danharrin
Copy link
Member

danharrin commented Nov 25, 2023

Please use $isSubNavigationAlignedEnd, isSubNavigationAlignedEnd() in methods and properties

Including $isSubNavigationAlignedEnd in the view $isSubNavigationAlignedEnd = $this->isSubNavigationAlignedEnd();

@zepfietje
Copy link
Member

@danharrin @glennjacobs wouldn't it be nicer to have an alignment property instead (start/end)?
Feels a little nicer to me than a boolean.

@danharrin
Copy link
Member

Yeah could be a good idea, we can use the alignment enum

@zepfietje
Copy link
Member

Yeah could be a good idea, we can use the alignment enum

Yup, could you handle this, @glennjacobs?

@glennjacobs
Copy link
Contributor Author

But then it would be left and right, which didn't feel right to me considering RTL. I did consider it initially.

@glennjacobs
Copy link
Contributor Author

Oh, add start&end to the enum. Ok.

@glennjacobs
Copy link
Contributor Author

@zepfietje @danharrin all done.

@zepfietje zepfietje assigned zepfietje and unassigned danharrin Nov 27, 2023
@zepfietje
Copy link
Member

I'll take over this PR to merge it with the changes I've just made.

@glennjacobs
Copy link
Contributor Author

I'll take over this PR to merge it with the changes I've just made.

No worries. I'll keep an eye out so I understand.

@glennjacobs
Copy link
Contributor Author

@zepfietje I have noticed one issue which is when the navigation is aligned right, the mobile menu is then presented at the bottom of the screen. Yikes.

@zepfietje
Copy link
Member

Will take care of that, @glennjacobs.

@zepfietje zepfietje changed the title Allow sub-navigation to align right Allow sub-navigation to align end Nov 27, 2023
@zepfietje zepfietje changed the title Allow sub-navigation to align end Support sub-navigation position end Nov 27, 2023
@zepfietje
Copy link
Member

Alright, all done.
Thanks, @glennjacobs!

@zepfietje zepfietje merged commit a60c6fd into filamentphp:3.1 Nov 27, 2023
4 checks passed
@glennjacobs glennjacobs deleted the sub-navigation-align-right branch November 27, 2023 12:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

3 participants