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

feat(ui) move room info from tv to remote #1031

Merged
merged 1 commit into from
Sep 7, 2023
Merged

Conversation

mihhu
Copy link
Member

@mihhu mihhu commented Aug 24, 2023

No description provided.

@mihhu mihhu force-pushed the cleanup-spot-view branch 2 times, most recently from 1f8b4f5 to 33aa39d Compare August 29, 2023 14:13
Copy link
Member

@saghul saghul left a comment

Choose a reason for hiding this comment

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

Other than the meeting name we need to display the sharing domain and code.

Can you add a screenshot too please?

@@ -4,9 +4,6 @@ import PropTypes from 'prop-types';
import React from 'react';
import { connect } from 'react-redux';


import { JoinInfo } from './../components';
Copy link
Member

Choose a reason for hiding this comment

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

Is this component used elsewhere? If not, please kill it.

Copy link
Member Author

Choose a reason for hiding this comment

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

It's still used on the /tv page

@mihhu
Copy link
Member Author

mihhu commented Sep 5, 2023

Other than the meeting name we need to display the sharing domain and code.

Ohh sorry, I'll add the code too, any suggestion for the UX? I would've said another pipe next to the domain/room, but it's already packed as it is. Maybe on the line below in all caps and in a bolder font?

Can you add a screenshot too please?

spot-tv remote

@mihhu
Copy link
Member Author

mihhu commented Sep 5, 2023

image

Something like this maybe, with both the sharing domain and code next to the spot room name?

@saghul
Copy link
Member

saghul commented Sep 5, 2023

Do we need both URLs?i think just the one at the top is enough no?

@mihhu
Copy link
Member Author

mihhu commented Sep 6, 2023

image

How about this?

@mihhu mihhu requested a review from saghul September 7, 2023 10:22
@saghul
Copy link
Member

saghul commented Sep 7, 2023

image

How about this?

I'd drop the name of the room from under the meeting name, since it's already at the top. LGTM otherwise!

@mihhu mihhu merged commit 41677e5 into master Sep 7, 2023
5 checks passed
@mihhu mihhu deleted the cleanup-spot-view branch September 7, 2023 12:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants