Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update styling for controllers in mobile view #94

Merged
merged 18 commits into from Oct 3, 2023
Merged

Conversation

medied
Copy link
Contributor

@medied medied commented Sep 12, 2023

https://phabricator.wikimedia.org/T340660

This is mainly a styling change to update the mobile view of edit controllers (change, remove) for the preview of the Wikipedia Preview. In order to ensure the new dots menu is correctly positioned regardless of preview height, I opted for inserting it directly in the preview header (trying to position it on the fly was messy and not getting me anywhere). This is what the controllers look like on mobile now (1), including RTL support (2), and the desktop view is left unchanged (3).

Also note I'm adding the eslint exception because according to https://react.dev/reference/react/useState#updating-state-based-on-the-previous-state, we need to use an updater function for toggleControllersMenu to work correctly.

image

@medied medied marked this pull request as ready for review September 13, 2023 20:17
src/link/preview.js Outdated Show resolved Hide resolved
src/link/style.scss Outdated Show resolved Hide resolved
Copy link
Member

@hueitan hueitan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

review

@hueitan
Copy link
Member

hueitan commented Sep 21, 2023

rtl ltr
testing-purpose_wp-admin_post php_post=1 action=edit(iPhone 12 Pro) testing-purpose_wp-admin_post php_post=1 action=edit(iPhone 12 Pro) (1)

@stephanebisson stephanebisson merged commit 2c087d8 into main Oct 3, 2023
1 check passed
@stephanebisson stephanebisson deleted the T340660-toolbar branch October 3, 2023 16:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants