-
Notifications
You must be signed in to change notification settings - Fork 40
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
[UX] Move the Page title settings to a dialog. #952
Comments
Got this working at backdrop/backdrop@1.x...docwilmot:custom-regions. Tests to follow unless any recommendations against this. |
PR up. |
I left some feedback over at the PR. |
Just pushed a new PR that addresses all of the feedback posted by @klonos on the first PR by @docwilmot |
Tests failed, but I will give it a spin anyways. Just need a few hours to 💤 first 😄 |
@jenlampton can you please force the PR so we can get a sandbox? |
I was thinking (will file a separate follow-up) would it be possible and would it make sense to allow tokens to be used in the title field? |
For my own reference, the latest PR is at backdrop/backdrop#1284. I gave it a review and it looks great! Nice job @docwilmot and @jenlampton. The only thing needed here is the existing tests need to be updated. The existing functionally for titles is already tested, just needs fixing. And we need to expand the test coverage to test pulling the title of the page from a block, as that new functionality is also included in this PR. |
Rebased and re-pushed. Also, tests updated & expanded! |
...the PR sandbox throws several "Undefined index" for various regions + a "Invalid argument supplied for foreach()" ...details left as feedback in the PR. |
We'll have to push this to 1.5.0 I'm afraid. So close! We made a lot of great improvements to Layouts this release. |
Is this something that could go in before 1.5? It's a UX improvement :) |
Yes please. |
Rebased, and rerolled to work with the new colors in #1935. Holding off on updating terminology until we get consensus on that. Either PR can go in first. |
Merged backdrop/backdrop#1284 into 1.x for 1.5.0. I assigned @docwilmot credit for the commit as the original author. Great job! This looks great and is so so so much better than those awkward title settings at the top of the page. |
OMG guys, you're on fire!! Another major annoyance fixed 🎉 ...it's hard to keep up with youz. |
Followup from #373.
The title type selection in Layouts is inconsistent with the rest of the UI. All the other configurations are in modals, so I would love to see the "title" area of the page have a similar configure link, and a modal, where we can hide this title type business. (see screenshot below)
The page title also gets lost on the page. It should probably be an "area" and not just a string of text, this can be accomplished with a border (and/or a background), like so:
The text was updated successfully, but these errors were encountered: