Closed Bug 1192372 Opened 6 years ago Closed 6 years ago
Fixed width and height for Loop desktop drop down menu panel
As a user I see a consistently sized panel, so the visual experience is smooth. Acceptance Criteria: Recent discussion suggested height of 410px and width of 330px. FTU is mocked up and is different height and will be smaller as shown in the mock. Tech Checklist: X Add these values to the panel-specific CSS, make sure it's not colliding with other older rules. X update all the sizes in the examples in ui-showcase.jsx X fix the various scrollbars in the showcase to match the mock-ups. X visually compare to the mock-ups and make fix list of changes. X get ui-review X review remaining code X fix review changes X visually compare fx-team and patch ui-showcases X run functional tests on patched version X land remaining code X spin off RTL showcase problems X spin off vertical centering of room-list-loading spinner to new bug X spin off .room-entry-context-actions > .dropdown-menu and should the word "context" be changed or gotten rid of (it seems wrong) to a new bug. X spin off lack of difference between ui-showcase "error notification" and "error notification - authenticated" views; may indicate real bug X spin off bug to disable gravatar promo in all views where it normally wouldn't show up in ui-showcase, leaving it enabled in at least one view. X spin off no-way to dismiss error in panel on importing contact failure X spin off bugs mentioned in comment 29 X spin off bugs mentioned in comment 30
Need: Prevent Loop panels from re-sizing when changing between views. Result: All Loop panels should have a fixed size.
Please make it 410px height. Thanks!
Summary: Fixed height for all Loop desktop panels → Fixed height for Loop desktop drop down menu panel
In case I'm not around when folks do the tech checklist, I'd recommend investigating doing this via the xul/chrome part of the UI. We already do special things for the chat box, so we should be able to do something here. I'm thinking that if we drop the dynamic resizing of the panel, then we're likely to avoid any issues with that resizing. I could be proved wrong though!
Attachment #8647190 - Attachment is obsolete: true
We tried to align the tab icons with the tab title text. It maybe off by a pixel, this CSS is extremely fragile and hard to adjust, I would suggest changing the icon itself.
qe-verify step is step 5 of the user story.
Chris and I initially spent some time thinking about how to specify this in XUL to avoid jank, and it doesn't look too hard, but it's a distraction from getting the height fixed so that the subsequent visual refresh changes don't have to cope with that adjustment later, so we're spinning that out to bug
User Story: (updated)
Summary: Fixed height for Loop desktop drop down menu panel → Fixed width and height for Loop desktop drop down menu panel
Note: See Bug 1194021 comment 1 and bug 1194021 comment 3 - the FTU can be the same size as the main panels as well.
Issues with current showcase: - reset.css is not being loaded. - top border missing/not showing on context tile: apply to new-room-view class DONE - panel width is not 330px - create example with/out context element - increase .rooms-list height to fill between footer and context window modes - put .context element inside .room-list element - make standard
Update of Issues with showcase and reviewed with Sevaan: - reset.css is not being loaded - FIXED - top border missing/not showing on context tile: apply to new-room-view class - DONE - apply border and padding buttons below contacts list. - move any error message to floating element below tabs - DONE - panel width is not 330px - DONE - create example with/out context element - increase .rooms-list height to fill between footer and context window modes - renamed empty rooms wrapper to rooms instead of rooms-list - DONE
Update of Issues with showcase: - apply border and padding buttons below contacts list. - DONE - move any error message to floating element below tabs - MAY SPIN OUT AS SEPARATE BUG - adjust widths to either 100% or 300px with 15px gutters - DONE - create example with/out context element - DONE - Set width and height to panel child panel-container - DONE - Add flex box layout to panel-container and children: messages, tab-view-container and footer - DONE - Add layout to tab layout and children to fill available space - adjusting 15 examples for new layout.
Code change list: - Gravatar needs to be hidden in render function conditionally. - Contact-list needs to be a div element with div children elements not ul.
(In reply to Chris Rafuse from comment #12) > - Gravatar needs to be hidden in render function conditionally. What is this referring to? The promotion or the icon next to each contact? Also, are you able to attach a work-in-progress patch as things are done?
Ed: Gravatar promo and we discussed in demo that we should just leave it alone for now and make it fit with the layout. I will try to push what I have today but component testing errors pending and fixing contact-list not overflow scrolling or not setting height still exists. Newly found Issue: - Tab-view ul has slide-bar div as child - not valid and may be causing sub layout issues - need to fix (wrap with li or remove from ul).
Attachment #8647193 - Attachment is obsolete: true
Comment on attachment 8651900 [details] [diff] [review] Fixed width and height for Loop desktop drop down menu panel -Contacts still not fixed due to possible tab-view issue (see next point). -Need to fix tab-view slide-bar ul issue. -all other panels are fixed.
Attachment #8651900 - Flags: feedback?(mdeboer) → review?(mdeboer)
Comment on attachment 8652158 [details] [diff] [review] Fixed width and height for Loop desktop drop down menu panel - Fixed Contacts layouts with overflow hidden. - Fixed slide-bar to be li instead of div.
Attachment #8652158 - Flags: review?(mdeboer) → review?(standard8)
Chris, did you actually look at the panel with your patch applied? How do you think it looks? And can you attach a screenshot for ui-review? Thanks!
Dan was proposing to make the fixed panel dimensions work in the UI Showcase first. I will attach a screen shot of the UI showcase. I will have to set the panel pinning patch to the FF build and make a snap shot of the panel menu and will be the Linux version. We will need snaps from IOS and Windows. Font sizes will be adjusted by: https://bugzilla.mozilla.org/show_bug.cgi?id=1191392 and https://bugzilla.mozilla.org/show_bug.cgi?id=1175841. Please continue with the review in the meantime as this is to be UX reviewed in code for new bugs as discussed in Monday's bug triage meeting.
Discussed changes in UX Review, will fix and attach updated patch: - Vertically align the no conversations message in the middle of the given space. - Move gravatar promo ad under the contacts title and include with contacts list scrollable area.
Progress update on this massively expanding BUG: Upstream fixes and KNOWN BUGS not really related to fixed height adjustments but should be spun out into separate bugs: Contacts Panel 1) Contact context menu for edit, block remove contact is positioned above the contact and menu gets cut off(or placed behind) from the MY CONTACTS title at top of contacts tab. _________________________ 2) Contact search (if more than 7 contacts, search bar will appear), when typing the keyword will never show the no search results view as it removes the keyword or letter from the input(empties input) if the letter or word is not present in the contacts. May be by design or is bug. Need to clarify the user action. _________________________ 3) Ideally remove the contact-list-container surrounding contact-list-title, contact-list-wrapper and button group as it is a coding requirement of return and not of the implementation (is just extra div element that contacts-container serves after Gravatar is removed - simplify or reduce parts count). Features/benefits: This MAY solve the flex issue of having to use fixed heights. Less elements to debug and have to deal with. _________________________ Edit Contacts Views 4) Edit Contact view is not like Add Contact view - Add contact should be reused or related in the Edit Contact view and not reinventing the wheel. Features/benefits: Standardized components and not having to mix in external elements into what should be a fully encapsulated or modular component.
Comment on attachment 8656935 [details] Fixed Panel Size 330px by 410px Screenshots Hi Vicky, Please review as this bug is blocking a number of other bugs. If you need any other views please comment and I will try and provide these. Also see above bug comment on the know issues that will be pushed to other bugs. Thank you for your time on this. -Chris
Attachment #8656935 - Flags: feedback?
Attachment #8656935 - Flags: feedback? → feedback?(sfranks)
Comment on attachment 8656935 [details] Fixed Panel Size 330px by 410px Screenshots Should there be a line above the [Start a conversation] button when there's no context? Similarly there's the line above the empty contacts view's buttons.
Comment on attachment 8656935 [details] Fixed Panel Size 330px by 410px Screenshots Wow! This is looking good! There are still some minor polish to get done, but the height looks pretty good. Thanks for the job done!
Attachment #8656935 - Flags: ui-review?(vpg) → ui-review+
(In reply to Ed Lee :Mardak from comment #25) > Comment on attachment 8656935 [details] > Fixed Panel Size 330px by 410px Screenshots > > Should there be a line above the [Start a conversation] button when there's > no context? Similarly there's the line above the empty contacts view's > buttons. No, the line shouldn't be there. It should only appear when scrolling. Thanks!
Comment on attachment 8656935 [details] Fixed Panel Size 330px by 410px Screenshots Looking great! This view is excellent to compare everything. +r overall, but I think there are couple minor changes we could make/consider... The "My Contacts" font size seems bigger than the "Recent Conversations" one. We should match the latter. http://i.sevaan.com/image/1V0W152C3q1k There seems to be more padding above "Recent Conversations" than above "My Contacts"...we should match the Recent Convos padding. While we're fixing titles, these titles (Add/Edit Contacts) should match the "Recent Conversations" and "My Contacts" http://i.sevaan.com/image/1E1A282H0G0q
Attachment #8656935 - Flags: feedback?(sfranks) → feedback+
Other bugs to spin out: Gravatar promo should not show if we have contacts list is already shown. Bugs in comments in bugs 1 and 2 in comment 22, all in comment 27 and all in comment 28 should be spun out. So we can land this bug now.
Couple of bugs I came across and to be aware of: 1) Sign in and account settings web pages appeared blank - the tab opened in but were blank - may be down or not working currently at test time. 2) I have seen Conversation titles (string data) not appear or are blank but the room entry element is still there and the correct number of rooms are there. This is for both signed in and anonymous modes. This may be from me killing firefox with a CNTRL+C from the bash prompt and not closing the browser properly(close button or ALT+F4) and causing data loss but I think it is more than that.
Note that code review and fixups are happening at <https://github.com/chrafuse/gecko-dev/pull/2/>.
Comment on attachment 8658512 [details] [diff] [review] Fixed width and height for Loop desktop drop down menu panel Rebased to latest fx-team and attached this but had this error on run-all-loop-tests on CNTRL+C after waiting about a minute: ^Cmach interrupted by signal or user action. Stopping. 4:07.02 TEST_END: Thread-23 FAIL 4:07.02 LOG: Thread-23 INFO browser/components/loop/test/xpcshell/test_loopservice_token_validation.js failed or timed out, will retry. ----- Built, ran FF and did visual check and worked.
Attachment #8658512 - Flags: review?(dmose)
Ran test again on the squashed branch and passed.
Comment on attachment 8658512 [details] [diff] [review] Fixed width and height for Loop desktop drop down menu panel Review of attachment 8658512 [details] [diff] [review]: ----------------------------------------------------------------- r=dmose This wants to be landed at the same time as bug 1192372. I'll push them both together.
Attachment #8658512 - Flags: review?(dmose) → review+
One more time! The patch in bug 1194021 has been folded into the latest rev here, so once this patch lands, by itself, that bug will also be fixed.
OK, Weds AM for Chris and me will involve spinning off the various bugs still mentioned in the user story, prioritizing them, and figuring out what's next.
Verified using latest Firefox Developer Edition across platforms (Windows 7 64-bit, Mac OS X 10.10.5 and Ubuntu 14.04 32-bit) that the panel has the same height and width (410x330) no matter where I move the hello icon, after clicking Get started, after login with FxA, after scrollbar appears in conversation/contacts, after restart, after crash and other scenarios.
You need to log in before you can comment on or make changes to this bug.