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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add 'switch to instance' buttons next to instances in instance list. #945

Merged
merged 1 commit into from Feb 7, 2019

Conversation

Projects
None yet
2 participants
@blackle
Copy link
Contributor

blackle commented Feb 6, 2019

screenshot from 2019-02-05 23-57-08

I split SettingsListItem into SettingsListRow and SettingsListButton. The first is responsible for styling its immediate children to look like buttons and have the right padding and borders (so that neighbouring items in the row only have a 1px border between them.) SettingsListButton is responsible for rendering a text button with the special eliding style.

I don't know if this is the best way to do this in terms of structure. It sure makes _pages/settings/index.html more verbose...

Maybe I should make a separate component for a row in the instance list and leave the SettingsListItem alone 馃

<svg class="instance-switcher-button-svg">
<use xlink:href="{instance.name === $currentInstance ? '#fa-star' : '#fa-star-o'}" />
</svg>
</button>

This comment has been minimized.

@nolanlawson

nolanlawson Feb 7, 2019

Owner

This should have a "pressed" state and an aria label to indicate what it does. I also like using titles so people can hover and it says what the button does. (For instance, "switch to this instance" and "switch away from this instance".)

@nolanlawson

This comment has been minimized.

Copy link
Owner

nolanlawson commented Feb 7, 2019

I love it!! This is such a great idea, I wish I had thought of it myself. :) My only feedback is an accessibility issue, but I can fix that in this branch, no worries.

@blackle

This comment has been minimized.

Copy link
Contributor Author

blackle commented Feb 7, 2019

I can fix the issues. I'll do that tomorrow night!

@nolanlawson

This comment has been minimized.

Copy link
Owner

nolanlawson commented Feb 7, 2019

No worries, I took care of it! Appreciate it, though. :)

@nolanlawson nolanlawson merged commit 503378a into nolanlawson:master Feb 7, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@nolanlawson

This comment has been minimized.

Copy link
Owner

nolanlawson commented Feb 7, 2019

BTW please let me know your fediverse handle if you'd like to be mentioned by the @Pinafore@mastodon.technology account when I toot about the next release 馃槂

@blackle

This comment has been minimized.

Copy link
Contributor Author

blackle commented Feb 7, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment