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

Add group history page #10430

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Add group history page #10430

wants to merge 7 commits into from

Conversation

cl8n
Copy link
Member

@cl8n cl8n commented Aug 2, 2023

part 4~ resolves #6216

full page screenshots

the search form is basically same as featured artists track search and the events list is basically same as user profile recent activity area. the icon BGs light up with the group's color, if it has one.



This page is now available at https://osu.ppy.sh/groups/history.

@cl8n cl8n marked this pull request as ready for review September 22, 2023 04:00
@cl8n cl8n requested a review from nanaya September 22, 2023 04:00
Comment on lines +5 to +11
&__events {
display: flex;
flex-direction: column;
gap: 10px;
}

&__none {
Copy link
Collaborator

Choose a reason for hiding this comment

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

those are more like modifiers rather than elements.

return (
<form className={bn} data-loading-overlay='0' onSubmit={this.onSubmit}>
<div className={`${bn}__content ${bn}__content--inputs`}>
<InputContainer for='group' labelKey='group_history.form.group'>
Copy link
Collaborator

Choose a reason for hiding this comment

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

for breaks label target

Comment on lines +42 to +44
<div className={`${bn}__select-container`}>
<select
className={`${bn}__input`}
Copy link
Collaborator

Choose a reason for hiding this comment

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

interleaving the block hasn't been a good idea. this also caused repeating of the styling

{trans('group_history.form.group_all')}
</option>
{groupStore.all.map((group) => (
<option key={group.id} value={group.identifier}>
Copy link
Collaborator

Choose a reason for hiding this comment

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

the dropdown is unreadable with white/gray background. Also the dropdown arrow doesn't align quite right

import { action, computed, makeObservable, observable } from 'mobx';

class GroupStore {
@observable byId = observable.map<number, GroupJson>();
Copy link
Collaborator

Choose a reason for hiding this comment

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

observable.map can just be normal object

};

if ('playmodes' in event && event.playmodes != null) {
mappings.playmodes = transArray(
Copy link
Collaborator

Choose a reason for hiding this comment

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

should probably be called ruleset

Comment on lines +13 to +28
@icons: {
group-add: users;
group-remove: users-slash;
group-rename: users-cog;
user-add: user-plus;
user-add-playmodes: user-tag;
user-remove: user-minus;
user-remove-playmodes: user-tag;
user-set-default: user-cog;
};
each(@icons, {
.@{_top}--@{key} & {
@icon-var: 'fa-var-@{value}';
--icon: @@icon-var;
}
});
Copy link
Collaborator

Choose a reason for hiding this comment

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

set at block level? Also for the message bold styling

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

Successfully merging this pull request may close these issues.

Record and show group changes
2 participants