scratchfoundation / scratch-www

Standalone web client for Scratch
https://scratch.mit.edu
BSD 3-Clause "New" or "Revised" License
1.58k stars 832 forks source link

Manager demotion #5612

Open apple502j opened 3 years ago

apple502j commented 3 years ago

With the manager limit in place, existing studios with more than 40 managers have to remove a manager before adding new ones. Currently you can only remove from the studio, and you cannot demote i.e. make them curators.

This obviously have side effects:

There should be a way to demote a manager to a curator without removing and re-inviting. I hope Scratch Team actually improves a feature in the update.

LankyBox01 commented 3 years ago

yes

Marc92020 commented 3 years ago

yes also will the ST actually add this?

LankyBox01 commented 3 years ago

idk probably

ghost commented 3 years ago

We need this! It’s extremely efficient and useful, especially with the limit coming and more managers need to be “demoted”. This is something we actually want ST, so please add it!

paulkaplan commented 3 years ago

We've been sketching on this a bit. The manager tiles would get an additional item "Make Curator", and clicking it would remove them from the manager list, add them to the curator list, and trigger a success alert:

Screen Shot 2021-06-25 at 11 08 45 AM

Screen Shot 2021-06-25 at 11 08 58 AM

ghost commented 3 years ago

We've been sketching on this a bit. The manager tiles would get an additional item "Make Curator", and clicking it would remove them from the manager list, add them to the curator list, and trigger a success alert:

Screen Shot 2021-06-25 at 11 08 45 AM

Screen Shot 2021-06-25 at 11 08 58 AM

I think it would make more sense to have it say "Demote to curator" as the opposite of the "Promote to manager" item.

Accio1 commented 3 years ago

We've been sketching on this a bit. The manager tiles would get an additional item "Make Curator", and clicking it would remove them from the manager list, add them to the curator list, and trigger a success alert: Screen Shot 2021-06-25 at 11 08 45 AM Screen Shot 2021-06-25 at 11 08 58 AM

I think it would make more sense to have it say "Demote to curator" as the opposite of the "Promote to manager" item.

My guess would be that it isn't "Demote to curator" because it would have a negative connotation associated with it. For example, someone could see that someone was a curator who used to be a manager and then think/comment "Ha ha, ___ got demoted, they are such a loser" or something along those lines. On the other hand, just saying that they were "made a curator" removes that negative connotation and likely would result in less of the above thoughts/comments.

mxmou commented 3 years ago

I don't like the Make curator and is now a curator messages because I, and probably some other people, consider managers a subset of curators. The second message could be changed to is no longer a manager. About the first one and negative connotations: "Remove" sounds even more "negative", especially with the icon (maybe that could be changed to just an X? I don't think removing someone from a studio is similar in any way to throwing them into a trash can).

AmazingMech2418 commented 3 years ago

I don't like the Make curator and is now a curator messages because I, and probably some other people, consider managers a subset of curators. The second message could be changed to is no longer a manager. About the first one and negative connotations: "Remove" sounds even more "negative", especially with the icon (maybe that could be changed to just an X? I don't think removing someone from a studio is similar in any way to throwing them into a trash can).

I agree with the first part. Though, technically, it is being removed from the studio, so it's just stating a fact. And well, the trash can is kind of just a universal symbol of deletion.

paulkaplan commented 3 years ago

Hm I liked that suggestion @mxmou of "is no longer a manager", although my concern there is that it would be easy to think you accidentally clicked "Remove" instead of "Make curator", if you saw an alert that said "is no longer a manager". I think the "is now a curator" messaging helps with that. In general I'm not too concerned about wording, just because these messages display for a short time and the tiles will disappear and reappear on the other list, which should be quite noticeable.

In general, always strive to show over tell. In fact, it would be awesome to animate the situations where we shuffle tiles from one category to another. If @apple502j or anyone wants to experiment with motion/animation, i'd be very interested to see it. Thats the type of thing i find really hard with React that I wish I was still using jquery for...

Edit: if only everything was as easy as scratch ;) recording (11) (just kidding, but seriously some less crazy animation might help show whats happening)

apple502j commented 3 years ago

Not sure how that could work - what if the curator list was full? Where will they be inserted to?

While it's cool, I don't think it's possible - nor is useful. Certainly a good prank for the next year tho.

Accio1 commented 3 years ago

@paulkaplan maybe the tile that the manager being "demoted" is currently in could have a dissolve/fade out effect applied, then the page scrolls to the curators list where the tile fades back in in the correct place?

Edit: Here is an example I made in Scratch:

https://user-images.githubusercontent.com/48496940/123474487-06d91e80-d5c8-11eb-9cd1-fc91fd201853.mp4

AmazingMech2418 commented 3 years ago

Thats the type of thing i find really hard with React that I wish I was still using jquery for...

I think using inline CSS might work pretty well! Like it would be possible I think to use a state variable for the properties and create a temporary interval to change that state variable with a closure, and then of course the interval would clear itself on the end of the animation. If other elements need to be updated, there's probably something you could do with Redux for animation handling.

I'm about to experiment with this kind of thing right now!

AmazingMech2418 commented 3 years ago

@Accio1 I think it might be better if it also scrolls down the page to where it adds the user to the curator list. But anyways, I think that looks great!

Accio1 commented 3 years ago

@Accio1 I think it might be better if it also scrolls down the page to where it adds the user to the curator list. But anyways, I think that looks great!

Yes, I agree. I just didn't really have a bunch of time to do that in the mockup.

AmazingMech2418 commented 3 years ago

I was able to create a demo using states in React for animations, with an interval, and it worked!

https://user-images.githubusercontent.com/33498997/123477672-5e798900-d5cc-11eb-95fd-f60c994f6f8c.mp4

Just some basic React code in a component:


import React from "react";
import ReactDOM from "react-dom";

import "./Animate.css";

class Animate extends React.Component {
  constructor() {
    super();
    this.state = {
      x: 0,
      y: window.innerHeight / 2 - 25
    };
  }

  animate = () => {
    let i = 0;
    let interval = setInterval(() => {
      this.setState({x: this.state.x + 1});
      if(i > 100) {
        clearInterval(interval);
      }
      i++;
      }, 10);
  }

  render() {
    return <div className="animate" onClick={this.animate} style={
      {
        top: this.state.y,
        left: this.state.x
      }
      }></div>
  }
}

export default Animate;

Atom has some very weird default indentation rules though, but I was too lazy to change them

Something like this could possibly be used to animate the manager demotion.

cobaltt7 commented 3 years ago

What's aninate.css?


From: AmazingMech2418 @.> Sent: Friday, June 25, 2021 2:47:53 PM To: LLK/scratch-www @.> Cc: Subscribed @.***> Subject: Re: [LLK/scratch-www] Manager demotion (#5612)

I was able to create a demo using states in React for animations, with an interval, and it worked!

https://user-images.githubusercontent.com/33498997/123477672-5e798900-d5cc-11eb-95fd-f60c994f6f8c.mp4

Just some basic React code in a component:

import React from "react"; import ReactDOM from "react-dom";

import "./Animate.css";

class Animate extends React.Component { constructor() { super(); this.state = { x: 0, y: window.innerHeight / 2 - 25 }; }

animate = () => { let i = 0; let interval = setInterval(() => { this.setState({x: this.state.x + 1}); if(i > 100) { clearInterval(interval); } i++; }, 10); }

render() { return <div className="animate" onClick={this.animate} style={ { top: this.state.y, left: this.state.x } }>

} }

export default Animate;

Atom has some very weird default indentation rules though, but I was too lazy to change them

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHubhttps://github.com/LLK/scratch-www/issues/5612#issuecomment-868796273, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AOT5DEVKFAVM6IAR6H5C6I3TUTMOTANCNFSM46XIEUQA.

AmazingMech2418 commented 3 years ago

What's aninate.css?

Just a basic CSS file with this:

.animate {
  display: block;
  width: 100px;
  height: 50px;
  background-color: red;
  position: absolute;
  top: 0;
  left: 0;
}

Just for setting the size and color of the div element.

paulkaplan commented 3 years ago

@AmazingMech2418 nice! We do a fair amount with css animations around the code, see e.g. the "wiggle" animation when you share blocks to other sprites in the editor https://github.com/LLK/scratch-gui/blob/ba76db7350bd43b79119cac2701bc10f6c511f0c/src/components/sprite-selector/sprite-selector.css To do a movement animation you need to know ahead of time where you are moving to, and we don't have anything to support that kind of cross-component communication (see studio-curators.jsx and studio-managers.jsx for those components you'd talk between). You'd have to refactor the code entirely just to support that basically :(

@Accio1 I love the scratch-based demos, keep em coming. But the fades don't make it clear that you are specifically going from one section to another, especially if both lists are long and you can't necessarily see the end of the curator list where that tile will appear. Thats why I was thinking motion.

Now that I say that, maybe we should ditch the menu items entirely and just have the tiles be draggable like blocks...

AmazingMech2418 commented 3 years ago

@paulkaplan Do you think something like useRef to get the position of an element might work? And for knowing where to move to, it might be possible to estimate the positions relative to the curator list that all of the curators in the list would be and find the last one and move there. Like each curator in the list has certain dimensions and a certain amount of padding, and from there, you could calculate the position to move to, when you count in the available width for the list (which could then either use useRef again or just estimate based on window.innerWidth).

If absolutely needed, the references could even be stored on a shared state with Redux.

AmazingMech2418 commented 3 years ago

The primary issue that would remain is that CuratorTile and ManagerTile are separate components based on how they are connected with Redux. I'm not sure if there is a way to change this at runtime to transition a ManagerTile to a CuratorTile, but worst-case scenario, it probably wouldn't be super difficult to use a wrapper component with a prop like isManager or isCurator that determines in the render function which to display.

paulkaplan commented 3 years ago

@AmazingMech2418 yeah I think a ref, specifically you could render the list of tiles and then render one extra hidden "placeholder" tile that would act as an anchor for where any incoming tiles should target. A custom hook could play the role of global store if you didn't want it in redux. Maybe I'll try to make a demo over the weekend, I like the idea of a general purpose "target" provider...

jeffalo commented 3 years ago

Now that I say that, maybe we should ditch the menu items entirely and just have the tiles be draggable like blocks...

I think this would be a pretty interesting way to do it. It would fit in with the Scratch editor, and would be a neat touch. Perhaps there could be both systems at the same time? Draggable tiles but also the context menu incase the user doesn't know tiles can be dragged.

And draggable tiles could lead to some interesting results if it allows reordering of curators/projects..

AmazingMech2418 commented 3 years ago

I just tried to set up a local build to experiment some with the animations for the tiles, but it seems like the curator retrieval API for 3.0 hasn't been released yet. Any estimation on when it will be released or if there is anything I can use in place of the API for testing purposes?

jeffalo commented 3 years ago

I just tried to set up a local build to experiment some with the animations for the tiles, but it seems like the curator retrieval API for 3.0 hasn't been released yet. Any estimation on when it will be released or if there is anything I can use in place of the API for testing purposes?

It's restricted by IP via fastly. Not sure why though.

AmazingMech2418 commented 3 years ago

I just tried to set up a local build to experiment some with the animations for the tiles, but it seems like the curator retrieval API for 3.0 hasn't been released yet. Any estimation on when it will be released or if there is anything I can use in place of the API for testing purposes?

It's restricted by IP via fastly. Not sure why though.

Yeah, I was hoping to tinker with it some and make a wrapper component for ManagerTile and CuratorTile... But, if you can't even have the tiles in the first place, there's not really much to tinker with!

paulkaplan commented 3 years ago

@BryceLTaylor this is a good reminder we need to take off the Ip blocks on the production studio routes before launch. Would be bad to forget that!

paulkaplan commented 3 years ago

But yeah it's tough to do local development that needs real data. I think your best bet right now is to look at the shape of the data that's expected in the code and mock out the response. I think there are chrome extensions that help with that kind of thing without modifying the code, but not really sure. It's something I was hoping to make better but didn't get around to it (it's not great even for scratch team developers)

Shluffy commented 3 years ago

@Accio1 your mock-up shows no buttons or confirmations, which could be a problem. There should also be a demotion notification

Accio1 commented 3 years ago

@Accio1 your mock-up shows no buttons or confirmations, which could be a problem. There should also be a demotion notification

I was more mocking up the actual animation rather than the buttons that you would have to press to get there.

@Accio1 I love the scratch-based demos, keep em coming. But the fades don't make it clear that you are specifically going from one section to another, especially if both lists are long and you can't necessarily see the end of the curator list where that tile will appear. Thats why I was thinking motion.

Now that I say that, maybe we should ditch the menu items entirely and just have the tiles be draggable like blocks...

I was also thinking that if the page was long, it would scroll to the bottom before the tile appears. That way, the user's attention would be drawn to it. However, I agree that physical motion would likely be better at drawing the user's attention that the tile is changing places. I also really like the idea of dragging the tile to the new spot.

CubeyTheCube commented 3 years ago

Here's a mockup I made for a demotion animation

https://user-images.githubusercontent.com/72284516/123497089-921ed800-d5f9-11eb-89db-a48bd8cffc7f.MP4

AmazingMech2418 commented 3 years ago

If the demotion becomes an animation (assuming we don't switch to the drag and drop version), would promotion also become an animation or would it just stay the way it's been?

AmazingMech2418 commented 3 years ago

This is a fairly crude animation hook, but I think it could be adapted for the demotion animation.

function useAnimation() {
  const animationRef = useRef();
  const xRef = useRef();
  const yRef = useRef();
  const [x, setX] = useState(0);
  const [y, setY] = useState(0);
  const [animating, setAnimating] = useState(false);

  const stateRefs = useRef();
  stateRefs.current = {x, y};

  useEffect(() => {
      xRef.current = animationRef.current.offsetLeft;
      yRef.current = animationRef.current.offsetTop;
  })

  console.log(x, y);

  const animate = () => {
    const to = {x: 100, y: 100};
    const dx = to.x - xRef.current;
    const dy = to.y - yRef.current;

    setX(xRef.current);
    setY(yRef.current);
    console.log(xRef.current, yRef.current);
    setAnimating(true);

    let i = 0;
    const interval = setInterval(() => {
        const {x, y} = stateRefs.current;
      console.log(dx, dy);
      console.log(x, y)
      console.log(
      x + dx/100,
      y + dy/100);
      setX(x + dx/100);
      setY(y + dy/100);
        if(i > 100) {
          clearInterval(interval);
          setAnimating(false);
        }
        i++
    }, 10);
  }

  return {animationRef, pos: {x, y}, animating, animate};
}

const StudioMemberTile = ({
    canRemove, canPromote, onRemove, onPromote, isCreator, hasReachedManagerLimit, // mapState props
    username, image // own props
}) => {
    const [submitting, setSubmitting] = useState(false);
    const [modalOpen, setModalOpen] = useState(false);
    const [managerLimitReached, setManagerLimitReached] = useState(false);
    const {errorAlert, successAlert} = useAlertContext();

    const {animationRef, pos, animating, animate} = useAnimation();

    const style = {};
    if(animating) {
      style.position = "absolute";
      style.zIndex = "999";
      style.left = pos.x;
      style.top = pos.y;
    }

    const userUrl = `/users/${username}`;
    return (
        <div className="studio-member-tile"
          ref={animationRef}
          style={style}
        >
            <a href={userUrl}>
                <img
                    className="studio-member-image"
                    src={image}
                />
            </a>
            <div className="studio-member-info">
                <a
                    href={userUrl}
                    className="studio-member-name"
                >{username}</a>
                {isCreator && <div className="studio-member-role"><FormattedMessage id="studio.creatorRole" /></div>}
            </div>
            {(canRemove || canPromote) &&
                <OverflowMenu>
                    {true && <li>
                        <button
                            onClick={() => {
                                animate();
                            }}
                        >
                            <img src={promoteIcon} />
                            <FormattedMessage id="studio.demote" />
                        </button>
                    </li>}
 /* More is below this, but I'm cutting it off here since we don't need everything else */

Here is what this looks like:

https://user-images.githubusercontent.com/33498997/123522942-b24ca680-d68e-11eb-93fb-47089f459657.mp4

This demo doesn't include it "knowing" where to go to or the placeholder needed so that the rest of the tiles know to stay where they are, but at least it shows this sort of animation is possible. For this example, I just made the tiles to go 100, 100.

Also, of course, we don't want the tile to return to where it was after the animation, but to trade places with the placeholder in the curators section, so this would also have to be done.

AmazingMech2418 commented 3 years ago

@paulkaplan For the placeholders, do you think a normal tile, just with the visibility: none property would work?

What I'm thinking currently is to have a placeholder in the curators list that, after the animation, will simply take on the properties of the manager tile being animated. This placeholder would also use a custom hook to send the position to the manager tile so it knows where to animate to. The only issue here though is, how do we add a new placeholder after this one when the animation is complete?

Also, I think to save the position of the manager tile so the other tiles don't just fill in, there could be a placeholder div that is activated when the animation is on (since the actual tile div is what is animated, not the entire component) and when the animation is complete, the style width is just set to 0 and there is a transition-duration to animate this to create a smoother fill-in of that space after the animation.

AmazingMech2418 commented 3 years ago

I used the placeholder thing with the extra div and it seems to have worked!

https://user-images.githubusercontent.com/33498997/123523717-63edd680-d693-11eb-9211-6bc1e783078a.mp4

I also added a small optional prop for the overflow menu so we can get it to close on animation, and I also preserved the width by storing that in a ref along with xRef and yRef. I think I'm going to store the code I've edited in a gist so it's easier to share updates since I don't have this local clone linked to a repo.

Edit: I just remembered gists only support 1 file at a time, so I'm probably just going to just create a branch of my fork.

Another edit: See the code here: https://github.com/AmazingMech2418/scratch-www/tree/animation

AmazingMech2418 commented 3 years ago

I just added so that the tile grows and shrinks back:

https://user-images.githubusercontent.com/33498997/123524959-94397300-d69b-11eb-9e18-6818af944af7.mp4

ghost commented 3 years ago

So if I understand, the managers limit will be to 40?

edit* and when the 3.0 studios will be out?

AmazingMech2418 commented 3 years ago

So if I understand, the managers limit will be to 40?

Well, the manager limit is https://github.com/LLK/scratch-www/pull/5588, not here.

lankybox02 commented 2 years ago

a

I made a simple code for the 3.0 page that includes a demote button for the dropdown, a modal, a simple fading animation and a success alert.

The "lanky2" tile is supposed to pop up in the curator section & the modal header should include the demoted manager curator image

ilikecereal1legacy commented 1 year ago

any updates?