enh(core): move all contacts menu features to unified settings #42662
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
emoral435
force-pushed
the
enh/core/42386/move-all-contacts-menu-features-to-unified-settings
branch
2 times, most recently
from
5e8916d to
74edbd4
Compare
emoral435
changed the title
emoral435
added
3. to review
emoral435
requested review from
nfebe,
skjnldsv,
ShGKme,
marcoambrosini and
nimishavijay
Signed-off-by: Eduardo Morales <emoral435@gmail.com>
Signed-off-by: Eduardo Morales <emoral435@gmail.com>
Signed-off-by: Eduardo Morales <emoral435@gmail.com>
emoral435
force-pushed
the
enh/core/42386/move-all-contacts-menu-features-to-unified-settings
branch
from
74edbd4 to
2974e33
Compare
|
Any idea on how to change the Cypress issues failing? They seem related to me removing the ContactsMenu.vue file :) |
There was a problem hiding this comment.
@emoral435 the functionality should be moved to the contacts search results, not to the search filters. I'm available tomorrow for a quick call to clarify this if you'd like :)
|
See the original issue to see resolution, and why this PR was closed! |
skjnldsv
deleted the
enh/core/42386/move-all-contacts-menu-features-to-unified-settings
branch
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.
Summary
Because the unified search has the same functionality as the contacts menu in the header, it makes more sense to put it in the unified search section of 'People'. This got cleared by the designers too! So I added the same functionality seen in the contacts menu. It essentially just 'moved' the functionality to the unified search :)
There are some caveats, 'sins of the father' per say that happened when moving the feature.
For example, as shown in the gif below, on scroll, the 'local time' popup can be scrolled in such a way that makes it go to high up in the view. Perhaps a scroll event can be used to stop this?
The action buttons are also not as highlighted on hover, as the whole button has to be hovered! There also may be some accessibility issues that are present, but in the last check, Micheal did not seem to find any, but if there are any, I will fix them as so.
As this is a medium sized commit, should I also include tests? New to this, just let me know :)
GIF to show the new changes!
Checklist