Put the children of AppNavigationItem outside of the main item #2704
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In the current implementation, a side effect is that it disables the ability to pass class="active" directly to the AppNavigationItem. Signed-off-by: Quentin Guidée <[email protected]> Update src/components/AppNavigationItem/AppNavigationItem.vue Update src/components/AppNavigationItem/AppNavigationItem.vue
Signed-off-by: Raimund Schlüßler <[email protected]>
Signed-off-by: Raimund Schlüßler <[email protected]>
Signed-off-by: Raimund Schlüßler <[email protected]>
raimund-schluessler
requested review from
marcoambrosini,
GretaD and
quentinguidee
raimund-schluessler
added
enhancement
|
@GretaD I fixed some more issues (collapsing didn't work, children were not styled correctly, pinning was broken, see my commits). I would suggest you test it again with Mail, and ideally with Calendar as well, probably. |
Signed-off-by: Raimund Schlüßler <[email protected]>
|
tested with mail and looks good, but i still have this on my code
|
Hm, no idea. Maybe this depends on the browser or how the app is compiled 🤔 When I use it in Tasks, it just creates a |
GretaD
approved these changes
May 25, 2022
2 tasks
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Originally created by @quentinguidee in #2572:
This change is necessary for #2571 (see why in action: #2571 (comment)). It puts AppNavigationItem's children out of the parent item, and displays them separately. This is the simplest implementation I found without recoding the entire AppNavigationItem. I'm open to ideas if you find something cleaner.
before.mov
after.mov
Important notes/questions to reviewers
In the current implementation, a side effect is that it disables the ability to pass (like in the doc)
class="active"directly to the AppNavigationItem. It seems like the only purpose of passing theclass="active"directly is only for the example? TheisActiveproperty works fine and continues to push the blue background.As another side effect of this PR, when hovering a child, it doesn't display the hover background of the parent (as demonstrated in the screenshots). I can go back to the first one easily if needed. Just unsure if it was a bug or not.