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.

Sign up for GitHub

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

Jump to bottom

Add record count in dns record table #509

Merged
merged 1 commit into from
Apr 4, 2023
Merged

Add record count in dns record table #509

merged 1 commit into from
Apr 4, 2023

Conversation

Genne23v
Copy link
Contributor

@Genne23v Genne23v commented Apr 3, 2023

Close #111

Add user DNS record count / limit in DNS table.

Screenshot 2023-04-03 at 2 13 30 PM

@Genne23v Genne23v added this to the Milestone 0.9 milestone Apr 3, 2023
@Genne23v Genne23v self-assigned this Apr 3, 2023
@Genne23v Genne23v added the category: front end Front end part of our web service label Apr 3, 2023
@humphd
Copy link
Contributor

humphd commented Apr 4, 2023

I would flip the order: put the text on the left, button on the right.

You could also play with https://chakra-ui.com/docs/components/stat/usage, which might look nice.

@Genne23v Genne23v requested a review from humphd April 4, 2023 14:30
@humphd
Copy link
Contributor

humphd commented Apr 4, 2023

@Genne23v can you do updated screenshots of this for reviewers?

@Genne23v Genne23v force-pushed the issue-111 branch 3 times, most recently from 9293716 to 45abed9 Compare April 4, 2023 15:29
@Genne23v
Copy link
Contributor Author

Genne23v commented Apr 4, 2023

I updated a bit more styling. Here's what it looks.

Screenshot 2023-04-04 at 11 27 39 AM

Screenshot 2023-04-04 at 11 28 22 AM

humphd
humphd requested changes Apr 4, 2023
View reviewed changes
Copy link
Contributor

@humphd humphd left a comment

Choose a reason for hiding this comment

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

Love this! One suggestion.

app/routes/__index/dns-records/index.tsx Outdated Show resolved Hide resolved
@Genne23v
Copy link
Contributor Author

Genne23v commented Apr 4, 2023

Here's the new look!

Screenshot 2023-04-04 at 11 35 17 AM

@Genne23v Genne23v requested a review from humphd April 4, 2023 15:37
humphd
humphd requested changes Apr 4, 2023
View reviewed changes
app/routes/__index/dns-records/index.tsx Outdated Show resolved Hide resolved
app/routes/__index/dns-records/index.tsx Outdated Show resolved Hide resolved
@Eakam1007
Copy link
Contributor

This is just a personal opinion, but would it be better to change the text to something like You have 1/20 maximum DNS Records

@Genne23v
Copy link
Contributor Author

Genne23v commented Apr 4, 2023

This is just a personal opinion, but would it be better to change the text to something like You have 1/20 maximum DNS Records

That would look like this.

Screenshot 2023-04-04 at 1 07 51 PM

@humphd @Eakam1007 let me know which one is preferred.

@Eakam1007
Copy link
Contributor

Hmm, the other style was definitely easier to read

@Genne23v
Copy link
Contributor Author

Genne23v commented Apr 4, 2023

Hmm, the other style was definitely easier to read

Thanks for the suggestion and quick feedback. I will keep Stat layout as I feel the same.

@Genne23v Genne23v requested a review from humphd April 4, 2023 17:20
@Genne23v Genne23v merged commit fc4fdca into DevelopingSpace:main Apr 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@humphd humphd humphd approved these changes

@Eakam1007 Eakam1007 Awaiting requested review from Eakam1007

@SerpentBytes SerpentBytes Awaiting requested review from SerpentBytes

Assignees

@Genne23v Genne23v

Labels
category: front end Front end part of our web service
Projects
None yet
Milestone
Milestone 0.9
Development

Successfully merging this pull request may close these issues.

Indicate how many more DNS records a user can create in the UI
3 participants
@Genne23v @humphd @Eakam1007