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.

Sign up for GitHub

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

Jump to bottom

compose: Move compose action buttons to a new popover. #12538

Closed
wants to merge 1 commit into from
Closed

compose: Move compose action buttons to a new popover. #12538

wants to merge 1 commit into from

Conversation

vinitS101
Copy link
Member

@vinitS101 vinitS101 commented Jun 8, 2019 •
edited

Move the 'New topic' and 'New private message' to a popover added after the reply button.

Additional changes to the text of these buttons depending on the narrow the user is in.
Resolves #12496

Update:
Normal window:
Screenshot from 2019-07-03 02-43-10

Mobile browser view:
Screenshot from 2019-07-03 02-43-18

@vinitS101
Copy link
Member Author

vinitS101 commented Jun 9, 2019 via email

@rishig
Copy link
Member

rishig commented Jun 9, 2019

Thanks @vinitS101!
There must be some code somewhere already that decides whether to prefill the stream (e.g. when you press the c shortcut). It would be great to reuse that code.

@zulipbot zulipbot added size: XL and removed size: S labels Jun 14, 2019
@vinitS101 vinitS101 changed the title [WIP] compose: Only show "New Topic" when in stream or topic narrow. [WIP] compose: Move compose action buttons to a new popover. Jun 14, 2019
@vinitS101 vinitS101 force-pushed the reply_button branch 8 times, most recently from 952ee2a to d832ba2 Compare June 19, 2019 22:32
@vinitS101 vinitS101 changed the title [WIP] compose: Move compose action buttons to a new popover. compose: Move compose action buttons to a new popover. Jun 19, 2019
@vinitS101 vinitS101 force-pushed the reply_button branch 6 times, most recently from de93a1d to 0664698 Compare June 21, 2019 23:52
@vinitS101 vinitS101 force-pushed the reply_button branch 2 times, most recently from ae1f791 to b72a1a4 Compare July 2, 2019 21:12
@vinitS101
Copy link
Member Author

vinitS101 commented Jun 16, 2020 •
edited

Rebased to remove conflicts.
@timabbott can you please review this?

Tests passing.

circle_reply_btn

@vinitS101 vinitS101 force-pushed the reply_button branch 2 times, most recently from 0ff91ba to 4b2002d Compare July 7, 2020 12:10
@timabbott
Copy link
Sponsor Member

@vinitS101 can you rebase this once more and fix the tests? We've moved that test suite to puppeteer, so might take a bit of porting but overall puppeteer is simpler :)

@vinitS101
compose: Move compose action buttons to a new popover.
bdb1b31 
Move the 'New topic' and 'New private message' to a popover added after
the reply button.
The text for these buttons is also dynamic now and changes with the
narrow.
Eg: 'New topic in #general' when in #general.

This also removes the 'mobile_message_buttons_popover' as it is now
redundant.

Resolves zulip#12496.
@zulipbot
Copy link
Member

Heads up @vinitS101, we just merged some commits that conflict with the changes your made in this pull request! You can review this repository's recent commits to see where the conflicts occur. Please rebase your feature branch against the upstream/master branch and resolve your pull request's merge conflicts accordingly.

@timabbott
Copy link
Sponsor Member

@vinitS101 it seems to be failing CI; note also that if you rebase, we've ported the Casper tests to Puppeteer (which I expect you'll find a lot nicer to deal with). See https://zulip.readthedocs.io/en/latest/testing/testing-with-puppeteer.html

@timabbott timabbott mentioned this pull request Mar 19, 2021
Closed
@alya alya added the redesign Part of the webapp redesign project, including blockers. label Dec 13, 2022
@zulipbot
Copy link
Member

Hello @zulip/design members, this pull request was labeled with the "redesign" label, so you may want to check it out!

@timabbott
Copy link
Sponsor Member

Closing as #12496 is no longer the current design plan; thanks for all your work towards this @vinitS101!

@timabbott timabbott closed this Sep 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rishig rishig rishig left review comments

@aero31aero aero31aero aero31aero left review comments

Assignees
No one assigned
Labels
has conflicts redesign Part of the webapp redesign project, including blockers. size: XL
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

compose: Only show "New Topic" when in stream or topic narrow.
6 participants
@vinitS101 @rishig @timabbott @zulipbot @aero31aero @alya