Closed
Bug 1192372
Opened 9 years ago
Closed 9 years ago
Fixed width and height for Loop desktop drop down menu panel
Categories
(Hello (Loop) :: Client, defect, P1)
Hello (Loop)
Client
Tracking
(firefox43 verified)
Tracking | Status | |
---|---|---|
firefox43 | --- | verified |
People
(Reporter: andreio, Assigned: crafuse)
References
Details
(Whiteboard: [visual refresh])
User Story
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
Attachments
(2 files, 4 obsolete files)
332.40 KB,
image/png
|
vicky
:
ui-review+
sevaan
:
feedback+
|
Details |
64.09 KB,
patch
|
dmosedale
:
review+
|
Details | Diff | Splinter Review |
Need: Prevent Loop panels from re-sizing when changing between views. Result: All Loop panels should have a fixed size.
Reporter | ||
Updated•9 years ago
|
User Story: (updated)
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → chris.rafuse
Comment 1•9 years ago
|
||
Please make it 410px height. Thanks!
Assignee | ||
Updated•9 years ago
|
Summary: Fixed height for all Loop desktop panels → Fixed height for Loop desktop drop down menu panel
Comment 2•9 years ago
|
||
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!
Assignee | ||
Comment 3•9 years ago
|
||
Assignee | ||
Updated•9 years ago
|
Attachment #8647190 -
Attachment is obsolete: true
Assignee | ||
Comment 4•9 years ago
|
||
Assignee | ||
Comment 5•9 years ago
|
||
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.
Assignee | ||
Updated•9 years ago
|
User Story: (updated)
Assignee | ||
Updated•9 years ago
|
User Story: (updated)
Assignee | ||
Updated•9 years ago
|
User Story: (updated)
Priority: -- → P1
Assignee | ||
Updated•9 years ago
|
User Story: (updated)
Updated•9 years ago
|
Flags: qe-verify+
Flags: firefox-backlog+
Comment 6•9 years ago
|
||
qe-verify step is step 5 of the user story.
Comment 7•9 years ago
|
||
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
Updated•9 years ago
|
Rank: 17
Comment 8•9 years ago
|
||
Note: See Bug 1194021 comment 1 and bug 1194021 comment 3 - the FTU can be the same size as the main panels as well.
Assignee | ||
Comment 9•9 years ago
|
||
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
Assignee | ||
Comment 10•9 years ago
|
||
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
Assignee | ||
Comment 11•9 years ago
|
||
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.
Assignee | ||
Updated•9 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Comment 12•9 years ago
|
||
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.
Comment 13•9 years ago
|
||
(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?
Updated•9 years ago
|
Rank: 17 → 14
Updated•9 years ago
|
Iteration: --- → 43.2 - Sep 7
Assignee | ||
Comment 14•9 years ago
|
||
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).
Assignee | ||
Comment 15•9 years ago
|
||
Assignee | ||
Updated•9 years ago
|
Attachment #8647193 -
Attachment is obsolete: true
Assignee | ||
Comment 16•9 years ago
|
||
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: review?(andrei.br92)
Attachment #8651900 -
Flags: feedback?(mdeboer)
Assignee | ||
Updated•9 years ago
|
Attachment #8651900 -
Flags: feedback?(mdeboer) → review?(mdeboer)
Assignee | ||
Comment 17•9 years ago
|
||
Assignee | ||
Updated•9 years ago
|
Attachment #8651900 -
Attachment is obsolete: true
Attachment #8651900 -
Flags: review?(mdeboer)
Attachment #8651900 -
Flags: review?(andrei.br92)
Assignee | ||
Comment 18•9 years ago
|
||
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)
Attachment #8652158 -
Flags: review?(andrei.br92)
Assignee | ||
Updated•9 years ago
|
Attachment #8652158 -
Flags: review?(mdeboer) → review?(standard8)
Comment 19•9 years ago
|
||
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!
Updated•9 years ago
|
Flags: needinfo?(chris.rafuse)
Assignee | ||
Comment 20•9 years ago
|
||
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.
Flags: needinfo?(chris.rafuse)
Assignee | ||
Comment 21•9 years ago
|
||
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.
Assignee | ||
Updated•9 years ago
|
Attachment #8652158 -
Attachment is obsolete: true
Attachment #8652158 -
Flags: review?(standard8)
Attachment #8652158 -
Flags: review?(andrei.br92)
Assignee | ||
Comment 22•9 years ago
|
||
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.
Assignee | ||
Comment 23•9 years ago
|
||
Attachment #8656935 -
Flags: ui-review?(vpg)
Assignee | ||
Comment 24•9 years ago
|
||
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?
Assignee | ||
Updated•9 years ago
|
Attachment #8656935 -
Flags: feedback? → feedback?(sfranks)
Comment 25•9 years ago
|
||
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 26•9 years ago
|
||
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+
Comment 27•9 years ago
|
||
(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 28•9 years ago
|
||
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+
Assignee | ||
Comment 29•9 years ago
|
||
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.
Assignee | ||
Comment 30•9 years ago
|
||
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.
Updated•9 years ago
|
Rank: 14 → 5
Updated•9 years ago
|
Iteration: 43.2 - Sep 7 → 43.3 - Sep 21
Updated•9 years ago
|
User Story: (updated)
Comment 31•9 years ago
|
||
Note that code review and fixups are happening at <https://github.com/chrafuse/gecko-dev/pull/2/>.
Updated•9 years ago
|
User Story: (updated)
Updated•9 years ago
|
User Story: (updated)
Assignee | ||
Updated•9 years ago
|
User Story: (updated)
Assignee | ||
Comment 32•9 years ago
|
||
Assignee | ||
Comment 33•9 years ago
|
||
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)
Assignee | ||
Comment 34•9 years ago
|
||
Ran test again on the squashed branch and passed.
Comment 35•9 years ago
|
||
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+
Comment 36•9 years ago
|
||
By which I actually meant, bug 11904021.
Comment 37•9 years ago
|
||
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.
Updated•9 years ago
|
User Story: (updated)
Updated•9 years ago
|
User Story: (updated)
Comment 39•9 years ago
|
||
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.
Comment 40•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/a11de304f968
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
status-firefox43:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
Updated•9 years ago
|
QA Contact: bogdan.maris
Assignee | ||
Updated•9 years ago
|
User Story: (updated)
Updated•9 years ago
|
User Story: (updated)
Updated•9 years ago
|
User Story: (updated)
Updated•9 years ago
|
User Story: (updated)
Updated•9 years ago
|
User Story: (updated)
Assignee | ||
Updated•9 years ago
|
User Story: (updated)
Comment 41•9 years ago
|
||
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.
Description
•