firefox-ios
firefox-ios copied to clipboard
mozilla-mobile
Proof of Concept: Page Zoom (FXIOS-1157 / #7596)
This is a working proof of concept for page zoom (#7596). Currently it uses the same zoom levels as Safari, but can be changed to match desktop. This needs UX and the strings put into Strings.swift before landing but otherwise should be working. (The version of the menu in the screenshots is not part of this PR)
| 
| 
|
| Menu at 100% | Page zoomed to 75% | Menu at 75% |
Recently submitted this as a bug. Would really enjoy using Firefox on my iPhone, but cannot due to this feature being absent.
Nice to see somebody else rocking the 75% to get more information on screen.
Far as the UX is concerned, it needs to be cleaned up. Whoever is maintaining the UI will hopefully incorporate this fix when revamping.
@q2r5 Hello! I was tasked to integrate zoom keyboard shortcuts and could take the opportunity to integrate some of your work in PR https://github.com/mozilla-mobile/firefox-ios/pull/9689. I cherry picked your Tab.swift zoom changes making sure it states your contribution. Please let me know if you think of a better way to do this, or if you see any improvement needed 🙏
This pull request has conflicts when rebasing. Could you fix it @q2r5?
![mergify[bot] avatar](https://avatars.githubusercontent.com/in/10562?v=4 "Published by a pub.dev verified publisher"){kind=small}
Hey @q2r5! I have some news about the #7596 ticket. The page action menu will be revisited soon and the zoom feature will be discussed on the design side at that point. If implemented, it looks like it would be in the design direction that @brampitoyo suggested in the ticket. It's not something we can easily do at the moment I believe and will need some development time. We can keep this PR opened in the meantime, but it won't be merged as is due to the design (and a product decision also needs to be made). Rest assured thought that this zoom feature is something we want as well! I hope this clears out the situation a little bit. Let me know if there's anything you wanna discuss on this topic!
This pull request has conflicts when rebasing. Could you fix it @q2r5?
@q2r5
Just to note, photon sheet rows can now contain more than one action. All actions are now defined in MainMenuActionHelper. The zoom actions would be three different SingleActionViewModel in a PhotonRowActions.
With the new menu, it’s currently looking like this. I’ll update the PR with this version after a bit of cleanup.
Looks great!
Although I think the controls are backwards. Plus on the right and - on the left?
That's great @q2r5 ! I'll bring this up with our designers so we can get some feedback 👀
This pull request has conflicts when rebasing. Could you fix it @q2r5?
This pull request has conflicts when rebasing. Could you fix it @q2r5?
@dnarcese @topotropic Hey. This PR looks like it's good to go and it addresses a feature that's requested often by many people. I think it would be amazing to get in. Do we have the greenlight to go ahead and merge it? If not, what's holding us back from doing so, and how can we get it done?
This pull request has conflicts when rebasing. Could you fix it @q2r5?
It seems the only hold up is "XCTAssertEqual failed: ("Google") is not equal to ("google-b-m") - incorrect for ["en-US", "en"] and US"
Why does this keep getting blocked? Would very much like to see this feature on iOS.
This pull request has conflicts when rebasing. Could you fix it @q2r5?
Why hasn’t this been implemented?
The UI was updated similar to Safari. But the adjustments seem to only affect plain text whereas Safari alters all rendering.
This is one of the oldest pull requests. Why not just repurpose the existing UI to target the rendering rather than plain text?
This keeps getting passed up each version. Why?
We are working on a larger project that involves adding this feature - target is to release before the end of the year.
@dnarcese OK. I suppose that's an answer. It's just baffling this wasn't prioritized as @q2r5 had coded the meaty bits 15 months ago.
This specific issue is fundamental towards usability & functionality on mobile screens. It's akin to outputting at 800x600 when the monitor is capable of 1080p. Quite an unpleasant browsing experience. That is the ultimate UX concern.
The focus on menu placement as reason to withhold critical functionality is quite absurd.
Skipped again in 104.
Mobile bookmark display priority & pasting “unformatted” shortcuts deemed more critical.
Skipped again in 105.
Clearly the Homepage layout was much more important to justify ignoring this for the 18th month.
Hi! We also just soft froze v106, I think @RespectIsEverything we can respectfully restrain another message on this topic for the v106 release as we already know it's not making it. I'm sure you can understand that although this topic looks simple from an external point of view (just merge the PR, duh), it has in fact underlying ramification internally and has a complexity of it's own (hence why we can't just merge this PR at the moment). Rest assured that this feature is on the radar and is being studied from a product perspective. In the meantime, webpage size can be increased or decreased through reader mode or through iPad keyboard shortcuts. Thanks for your understanding.
@lmarceau Changing the font size options in from the Reader Mode menu only alters the size of plain text This PR pertains to scale for rendering of all elements on a page.
Ramifications exist for every decision ever made. In an ideal world, Apple wouldn't lock browsers into using WebKit on iOS. In another Shangri-La, they'd make the rendering scale available via an API.
But unless something in Apple's terms prevents this feature from being implemented, the neglect has been a willful choice by Mozilla's iOS team.
The issue itself was reported well before @q2r5 uploaded this PR 18 months ago. If being reminded about this is bothersome, perhaps the best way to unburden ones' self is by prioritizing merge of the chronologically penultimate PR.
This pull request has conflicts when rebasing. Could you fix it @q2r5?
@dnarcese
We are working on a larger project that involves adding this feature - target is to release before the end of the year.
Skipped again in 107. This isn’t tagged for 108 or 109.
So either this larger project is akin to Madoff’s legendary returns or the target is presumably pushed back beyond any reasonable belief that this will ever be implemented.
Not a small bug that sporadically glitches. This functionality is critical to using the browser on such a small screen. It’s unconscionable this wasn’t prioritized from the start.
Hi @q2r5!
I am the assigned UX Designer for this feature.
Would you have the time to implement the feature with the following UX improvements?
Please read the product requirements for the feature on this issue.
If there are any questions, or if there is anything you need from me, please let me know on the issue itself.
Thank you!
Hello @q2r5, Just wanted to add some more thoughts on the update if you are still interested in working on the zoom feature. We will aim to split the feature into smaller tickets, so they are more easily achievable (and reviewable). You could enable some of those tickets, or all. It's really as you wish! We wanted to make sure you were involved since you've been an advocate for this feature since a really long time. Let us know if that interests you and we can create sub-tickets for this work. Thank you!