-
-
Notifications
You must be signed in to change notification settings - Fork 35
Redesign contact links #1511
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
base: main
Are you sure you want to change the base?
Redesign contact links #1511
Conversation
|
Re-tested this tonight after rebasing the branch. The one potential issue I can find is that switching to keyboard navigation can cause the page to scroll. Testing with small resolutions shows this is because the browser (at least for Firefox) wants to make the combobox be at the top of the page, if there is enough room. I'm unsure if this is an issue with my PR, or if this is expected behavior on the part of the browser. ^^; |
It's getting a bit long, and the future combobox component will show all recognized sites on click if scripting is available.
This comment was marked as outdated.
This comment was marked as outdated.
- add spacing between cells - avoid table width discontinuity over viewport widths - stop buttons from overflowing their cells - set vertical alignment on table headers for when their text wraps
…mpts to read it `main.js` loads asynchronously, so if it’s cached, it can upgrade the `<input is="weasyl-combobox">` before its `list` is in the DOM.
|
|
||
| this.#popover.style.top = `${top}px`; | ||
| this.#popover.style.left = `${left}px`; | ||
| this.#popover.style.maxHeight = `${rect.height * 6.7}px`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.7 to emphasize that it’s scrollable? Nice touch :)
When the field has history, it gets a datalist-like dropdown arrow and additional popover that eats keyboard input on Chrome, for example.
We probably still want a custom icon if we proceed with this icon-button approach, but I’m just noting that this exists.
to expose a hover tooltip. This doesn’t help all users, but it should be a straight improvement.
charmander
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Partial list of things that still need work (which I meant to get to sooner, but there’s a lot going on right now – will take care of eventually):
- Almost every site icon should be an SVG (Itaku and Inkbunny are acceptable exceptions), with an appropriate level of detail for the display size (Reddit is too detailed; Piczel’s SVG should be less detailed than its 300 KB PNG), and optimized. We should use SoFurry’s new logo.
- Safari doesn’t support extending built-in elements with
<input is="…">. Have we made an explicit decision about not supporting it? scrollIntoView(regardless of options, I think?) doesn’t behave the same way as native dropdown lists.- Table layout on narrow viewports. I don’t remember if I broke this by making it wider, but it’s important either way.
- We might want to implement a preview. It could apply sorting (highlight focused row) and display the URL to help catch mistakes. This could also be added later, though – maybe along with the future link verification feature.
- TODO: check dynamic fields’ interaction with bfcache/form state cache – is a
beforeunloadwarning more important on this page? - Is the disabled state clear enough for delete undo? We could hide the fields and replace the entire row with a
<td rowspan="3">Item deleted. <button class="link-button" type="button">Undo</button></td>. - We could suggest links to import from the profile text?
- Emoji/Unicode symbols in UI typically don’t look very professional. We’ll probably want to replace any remaining ones with SVG icons.
I’m going to push some changes directly here instead of opening a pull request for simplicity, but feel free to revert any.
| } | ||
|
|
||
| function addNewSocialGroupIfNeeded() { | ||
| if (this.querySelectorAll('input[value]')) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is always true.
Implements parts of #1477.
These changes will:
<input>+<datalist>approach remains the no-scripting fallback.New editor, scripting-enabled
New editor, no scripting
Not pictured:
<datalist>is still the user's friend.Published side
Notice the labels on two of the Bluesky links.